code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/cash/CashManager.sol#L755

Vulnerability details

Impact

Redeemers might be accidentally left out

Proof of Concept

The _processRedemption function has the quantityBurned as input parameter, which is the leftover from all cash burns from the desired round, after refunds have been deducted. Moreover the function has redeemers as parameter, which are all participants of the desired round that have requested a redemption. Therefore the quantityBurned parameter should be the sum of redemptionInfoPerEpoch[epochToService].addressToBurnAmt[redeemer]; for all redeemers. However, the function lacks a sanity check to ensure this is in fact the case. This might lead to undesired decreased redemptions in case of human error (partially wrong input).

Tools Used

VSCode

Recommended Mitigation Steps

Consider implementing a check that the sum of redemptionInfoPerEpoch[epochToService].addressToBurnAmt[redeemer]; for all redeemers indeed match quantityBurned amount to ensure no one is left out.

uint burnleft = quantityBurned;
loop {
     ...logic...
     uint burnLeft -= cashAmountReturned;
}
require(burnLeft == 0, "Burning amount mismatch, redeemer was forgotten");

This could be a way to implement the aforementioned sanity check.

trust1995 commented 1 year ago

Downgrade to QA due to conditional of wrong user input.

c4-judge commented 1 year ago

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