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

0 stars 0 forks source link

Improper Handling of Empty `expectedERC20sOut` Array in `redeemCustom()` #91

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#L320-L334 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/RToken.sol#L339-L343

Vulnerability details

The redeemCustom() function enables users to redeem their RTokens for a custom basket of underlying assets in specified proportions. However, there is an issue in the redeemCustom() function that prevents users from performing custom redemptions when the expectedERC20sOut array is empty, even if the redemption amounts are non-zero. This limits the flexibility of the custom redemption feature and may cause inconvenience for users who want to redeem their RTokens without specifying the expected output tokens.

The issue arises in the following code block in RToken.sol:320-334

bool allZero = true;
for (uint256 i = 0; i < erc20s.length; ++i) {
    if (amounts[i] == 0) continue; // unregistered ERC20s will have 0 amount
    if (allZero) allZero = false;
    // ...
}
if (allZero) revert("empty redemption");

The code checks if all the redemption amounts are zero and reverts with "empty redemption" if that's the case. However, when expectedERC20sOut is empty, the loop that checks the post-balances of the expected output tokens is skipped: RToken.sol:339-343

for (uint256 i = 0; i < expectedERC20sOut.length; ++i) {
    uint256 bal = IERC20(expectedERC20sOut[i]).balanceOf(recipient);
    require(bal - pastBals[i] >= minAmounts[i], "redemption below minimum");
}

As a result, the redeemCustom() function reverts with "empty redemption" even when the preconditions for a valid redemption are met.

Impact

Proof of Concept

Here is how.

Tools Used

Manual review

Recommended Mitigation Steps

To fix the issue and allow custom redemptions with an empty expectedERC20sOut array

// ...

bool allZero = true;
for (uint256 i = 0; i < erc20s.length; ++i) {
    if (amounts[i] == 0) continue; // unregistered ERC20s will have 0 amount
    if (allZero) allZero = false;
    // ...
}
- if (allZero) revert("empty redemption");
+ if (allZero && expectedERC20sOut.length > 0) revert("empty redemption");

// ...

for (uint256 i = 0; i < expectedERC20sOut.length; ++i) {
    uint256 bal = IERC20(expectedERC20sOut[i]).balanceOf(recipient);
    require(bal - pastBals[i] >= minAmounts[i], "redemption below minimum");
}

With these changes, the "empty redemption" revert will only occur if allZero is true and expectedERC20sOut is not empty. This allows custom redemptions to proceed when expectedERC20sOut is empty, as long as the redemption amounts are non-zero and other preconditions are met.

Assessed type

Error