Lines of code

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

Vulnerability details

Impact

The method completeRedemptions() accepts both the list of redeeming accounts that should be processed, and an amount to disburse among them as parameters. As iterating over ALL the redeeming accounts in a given epoch can exceed the block gas limit, it is assumed that this method will be called multiple times for a single epoch, each call processing a batch of redemption. However, there is no guarantee that there will be any integrity among the redemptions. See below for a scenario.

Proof of Concept

Assume the scenario when both Bob and Alice burned 100 CASH. The manager calls completeRedemptions() once with parameters [alice], 10 (here we have forgotten decimals on an ERC20 token) and once with parameters [bob], 10_000_000. The redemptions will be processed and Alice will receive much less for her burn of the same CASH amount as Bob.

Tools Used

None

Recommended Mitigation Steps

Make the processing of redemptions work with global numbers for the entire epoch processing. First, record the total amount of CASH burned. Then, record the total amount of collateral that will be distributed for the epoch. Then, transfer in the tokens, and process the transfers in batches working with the proper totals.

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