Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. setPendingRedemptionBalance() fails to modify currentRedeemAmount when epoch == currentEpoch. This is necessary since when epoch == currentEpoch, if redemptionInfoPerEpoch[epoch].addressToBurnAmt[user] is changed, then currentRedeemAmount needs to be changed too for consistency. Otherwise, one might redeem more over the minRedeem during the current 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. When epoch == currentEpoch, and this function is modifying redemptionInfoPerEpoch[epoch].addressToBurnAmt[user], we need to modify currentRedeemAmount accordingly so that redeemLimit can be respected. Otherwise, one might be able to redeem more tokens within the current epoch that is exceeding redeemLimit.

Tools Used

Remix

Recommended Mitigation Steps

The following code will modify currentRedeemAmount as well when the function will modify redemptionInfoPerEpoch[epoch].addressToBurnAmt[user] when epoch == currentEpoch:

function setPendingRedemptionBalance(
    address user,
    uint256 epoch,
    uint256 epoch
    uint256 newBalance
  ) external updateEpoch onlyRole(MANAGER_ADMIN) {
    if (epoch > currentEpoch) {
      revert CannotServiceFutureEpoch();
    }
    if(oldBalance != redemptionInfoPerEpoch[epoch].addressToBurnAmt[user]) revert WrongOldBalance(); // @audit

    // Increment or decrement total burned for the epoch based on whether we
    // are increasing or decreasing the balance
    if (newBalance < oldBalance) {
      redemptionInfoPerEpoch[epoch].totalBurned -= oldBalance - newBalance;
    } else {
      redemptionInfoPerEpoch[epoch].totalBurned += newBalance - oldBalance;
    }
    redemptionInfoPerEpoch[epoch].addressToBurnAmt[user] = newBalance;

    emit PendingRedemptionBalanceSet(
      user,
      epoch,
      newBalance,
      redemptionInfoPerEpoch[epoch].totalBurned
    );

   if(epoch == currentEpoch){  // @audit: currentRedeemAmount needs to be updated when epoch == currentEpoch
              if(newBalance > oldBalance) 
                       currentRedeemAmount += newBalance-oldBalance;
              else
                      currentRedeemAmount -= oldBalance - newBalance;
   }

}

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