code-423n4 / 2024-07-reserve-validation

0 stars 0 forks source link

Insufficient Validation in `redeemCustom()` Function Fails to Ensure Minimum ERC20 Token Redemption Amounts #93

Closed c4-bot-3 closed 1 month ago

c4-bot-3 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/RToken.sol#L338-L343

Vulnerability details

Users redeem their RToken balance for a custom basket of underlying assets through the redeemCustom() function. This function takes in parameters such as the recipient address, redemption amount, basket nonces, portions, expected ERC20 tokens, and minimum amounts. It performs various checks and effects, including refreshing the asset registry, verifying the redemption amount, applying throttles, and calculating the basket redemption amounts using the quoteCustomRedemption() function of the basketHandler.

The is issue in the post-checks section of the redeemCustom() function, specifically in the code block responsible for verifying that the received amounts of the expected output ERC20 tokens meet the specified minimum amounts (RToken.sol:338-343).

// Check post-balances
for (uint256 i = 0; i < expectedERC20sOut.length; ++i) {
    uint256 bal = IERC20(expectedERC20sOut[i]).balanceOf(recipient);
    // we haven't verified this ERC20 is registered but this is always a staticcall
    require(bal - pastBals[i] >= minAmounts[i], "redemption below minimum");
}

The code assumes that the recipient's balance of the expected ERC20 tokens will always increase by at least the specified minimum amounts. However, this assumption may not hold true in certain scenarios, such as when the backingManager does not have sufficient funds to fulfill the redemption request.

Impact

Proof of Concept

Consider this scenario

Tools Used

Manual review

Recommended Mitigation Steps

Handle the cases where the backingManager may not have sufficient funds to fulfill the redemption request. Instead of using a require statement, the code should provide appropriate error handling and allow for partial redemptions when the minimum amounts cannot be met.

// Check post-balances
for (uint256 i = 0; i < expectedERC20sOut.length; ++i) {
    uint256 bal = IERC20(expectedERC20sOut[i]).balanceOf(recipient);
    // we haven't verified this ERC20 is registered but this is always a staticcall
-   require(bal - pastBals[i] >= minAmounts[i], "redemption below minimum");
+   if (bal - pastBals[i] < minAmounts[i]) {
+       // Handle insufficient redemption, e.g., emit an event or revert with a specific error message
+       emit InsufficientRedemption(recipient, expectedERC20sOut[i], bal - pastBals[i], minAmounts[i]);
+   }
}

Assessed type

Error