setRedeemLimit() failes to compare __redeemLimit with currentRedeemAmount

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L609-L615

Vulnerability details

Impact

Detailed description of the impact of this finding. setRedeemLimit() fails to compare _redeemLimit with currentRedeemAmount. As a result, it is possible that currentRedeemAmount will exceed _redeemLimit for the current ongoing epoch.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. setRedeemLimit() fails to compare _redeemLimit with currentRedeemAmount. As a result, it is possible when a new _redeemLimit is set, currentRedeemAmount will exceed _redeemLimit for the current ongoing epoch.

One might argue that the new _redeemLimit will not take effect for the current epoch and will only take effect for the next epoch. This is not true, when requestRedemption() is called, the new _redeemLimit will be checked immediately, preventing future redemption when necessary. On the other hand, past violation of the new _redeemLimit for the current epoch is waived. Therefore, both old _redeemLimit and new _redeemLimit take effect on the current epoch. If this is part of the design, then a clear documentation is necessary. Otherwise, we need to check _redeemLimit with currentRedeemAmount to ensure that no violation has occurred so far before proceeding. Otherwise, setRedeemLimit() should be called at the beginning of next epoch when no violations have occurred.

Tools Used

Remix

Recommended Mitigation Steps

Our fix to do the check _redeemLimit with currentRedeemAmount to ensure no violations have occurred. If a violation has occurred, then wait for the beginning of next epoch to call setRedeemLimit():

function setRedeemLimit(
    uint256 _redeemLimit
  ) external onlyRole(MANAGER_ADMIN) {
   if(curerntRedeemAmount > _redeemLimit) revert currentRedeemAmountExceedRedeemLimitAlready(); // @ already violated

    uint256 oldRedeemLimit = redeemLimit;
    redeemLimit = _redeemLimit;
    emit RedeemLimitSet(oldRedeemLimit, _redeemLimit);
  }

[c4-judge]

Duplicate of https://github.com/code-423n4/2023-01-ondo-findings/issues/84

[c4-judge]

trust1995 marked the issue as grade-c

[c4-judge]

trust1995 marked the issue as grade-b