code-423n4 / 2024-05-loop-findings

4 stars 4 forks source link

Whitelisting Prevents Recovery of Mistakenly Sent Tokens via `recoverERC20` #94

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L380 https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L364

Vulnerability details

Impact

The current implementation of the recoverERC20 function (which is meant to recover mistakenly sent ERC20 token) will be unable to recover ERC20 tokens that are mistakenly sent to the contract If the token sent to the contract before it's whitelisted and then subsequently whitelisted using the allowToken function. The recoverERC20 function won't allow it to be recovered due to the check against isTokenAllowed. This can lead to a permanent loss of funds as the funds will be irrecoverable

Proof of Concept

1.ERC20 token A mistakenly sent to the contract.

2.Token A is not yet whitelisted, so it is not in isTokenAllowed.

3.Owner whitelists token A using allowToken.

4.Token A is now in isTokenAllowed.

5.Attempt to recover token A using recoverERC20 fails because isTokenAllowed[tokenA] is true.

Tools Used

Manual code review

Recommended Mitigation Steps

        function allowToken(address _token) external onlyAuthorized {
+        if (IERC20(_token).balanceOf(address(this)) > 0) {
+        recoverERC20(_token, IERC20(_token).balanceOf(address(this)));
         }
+        require(isTokenAllowed[_token] = false);
        isTokenAllowed[_token] = true;
    }

This will transfer out any mistakenly sent funds to owner before whitelisting and the require statement will ensure owner can just arbitrarily send already whitelisted token to himself from the contract

Assessed type

ERC20

c4-judge commented 3 months ago

koolexcrypto changed the severity to QA (Quality Assurance)

c4-judge commented 3 months ago

koolexcrypto marked the issue as grade-c