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

0 stars 0 forks source link

Some tokens revert if approval amounts are > type(uint96).max #82

Closed c4-bot-10 closed 1 month ago

c4-bot-10 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/BackingManager.sol#L73 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/libraries/Allowance.sol#L41

Vulnerability details

Impact

And they also change the allowance amount to type(uint96).max if the amount given is type(uint256).max

ERC20 token behaviors in scope

Proof of Concept

ERC20 token behaviors in scope shows that tokens that Revert on Large Approvals & Transfers are in scope. Some tokens (e.g. UNI, COMP) revert if the value passed to approve or transfer is larger than uint96.

BackingManagerP1::grantRTokenAllowance

    function grantRTokenAllowance(IERC20 erc20) external notFrozen {
        require(assetRegistry.isRegistered(erc20), "erc20 unregistered");
        // == Interaction ==
        IERC20(address(erc20)).safeApprove(address(rToken), 0);
 @@   IERC20(address(erc20)).safeApprove(address(rToken), type(uint256).max);
 }

AllowanceLib::safeApproveFallbackToMax

        function safeApproveFallbackToMax(
        address tokenAddress,
        address spender,
        uint256 value
 ) internal {
 IERC20ApproveOnly token = IERC20ApproveOnly(tokenAddress);

--------------

        // 3. Fall-back to setting a maximum allowance
        if (!success) {
 @@      token.approve(spender, type(uint256).max);
            // untestable:
            //    allowance should always be max value if token behaves correctly
            require(token.allowance(address(this), spender) >= value, "allowance missing");
 }
 }

Tools Used

Vs code

Recommended Mitigation Steps

Acknowledge that amount passed over type(uint96).max will only result in reverting and the system expects to use the amount in uint256. Don't allow UNI and COMP tokens to a backing token of rToken.

Assessed type

ERC20