Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. _processRefund() fails to updatecurrentRedeemAmountwhenepochToService == currentEpoch. As a result,currentRedeemAmount`` will contain the portion that has already been refunded, an incorrect number.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. When _processRefund() is called to refund some redeemers, it is important to check whether epochToService == currentEpoch, and if this is true, then we need to update currentRedeemAmount. In particular, we need to subtract from it totalCashAmountRefunded - the total amount of refunded CASH. In this way, the redeemLimit can properly checked in the future. Otherwise, currentRedeemAmount will contain the portion that has already been refunded, an incorrect number.

Tools Used

Remix

Recommended Mitigation Steps

The fix is to account for the refund for the value of currentRedeemAmount:

function _processRefund(
    address[] calldata refundees,
    uint256 epochToService
  ) private returns (uint256 totalCashAmountRefunded) {
    uint256 size = refundees.length;
    for (uint256 i = 0; i < size; ++i) {
      address refundee = refundees[i];
      uint256 cashAmountBurned = redemptionInfoPerEpoch[epochToService]
        .addressToBurnAmt[refundee];
      redemptionInfoPerEpoch[epochToService].addressToBurnAmt[refundee] = 0;
      cash.mint(refundee, cashAmountBurned);
      totalCashAmountRefunded += cashAmountBurned;
      emit RefundIssued(refundee, cashAmountBurned, epochToService);
    }

    if(epochToService == currentEpoch) 
            unchecked{currentRedeemAmount -= totalCashAmountRefunded ;} // @audit: correct currentRedeemAmount

    return totalCashAmountRefunded;
  }

code-423n4 / 2023-01-ondo-findings