`recoverERC20` token Failure if Owner is Blacklisted

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L379

Vulnerability details

Impact

The current implementation of the recoverERC20 function allows the owner to recover ERC20 tokens mistakenly sent to the contract. However, if the owner address is blacklisted, this function will always fail as it tries to transfer to token direct to the owner. As a result, any blacklisted tokens sent to the contract will be locked forever. making Blacklisted tokens, such as USDC, sent to the contract be irretrievable and locked forever. The inability to recover blacklisted tokens may lead to a loss of funds

Proof of Concept

based on the current implementation of the recoverERC20 function as below

    /**
     * @dev Allows the owner to recover other ERC20s mistakingly sent to this contract
     */
    function recoverERC20(address tokenAddress, uint256 tokenAmount) external onlyAuthorized {
        if (tokenAddress == address(lpETH) || isTokenAllowed[tokenAddress]) {
            revert NotValidToken();
        }
        IERC20(tokenAddress).safeTransfer(owner, tokenAmount);

        emit Recovered(tokenAddress, tokenAmount);
    }

Any attempt to recover USDC from the protocol will always revert if the owner address is blacklisted which will lead to loss of funds

Tools Used

Manual Review

Recommended Mitigation Steps

To address this issue, we recommend modifying the recoverERC20 function to accept an additional parameter _to, which specifies the destination address for the recovered tokens. This modification allows the contract owner to specify any address, regardless of whether it's the owner's address or not, as the destination for recovered tokens.

/**
 * @notice Allows the owner to recover other ERC20s mistakenly sent to this contract
 * @dev Restricted to be called by the owner only.
 * @param _tokenAddress Address of the ERC20 token
 * @param _tokenAmount Amount of ERC20 token to recover
 * @param _to Destination address to send recovered tokens
 */
function recoverERC20(address _tokenAddress, uint256 _tokenAmount, address _to) external onlyAuthorized {
    if (_tokenAddress == address(lpETH) || isTokenAllowed[_tokenAddress]) {
        revert NotValidToken();
    }
    IERC20(_tokenAddress).safeTransfer(_to, _tokenAmount);

    emit Recovered(_tokenAddress, _tokenAmount);
}

Assessed type

DoS

[0xd4n1el]

That could be a problem with blacklisted tokens, but we do not intend to do so according to the list of ERC20s posted

[c4-judge]

koolexcrypto marked the issue as unsatisfactory: Out of scope

[pratokko]

Hello, I would like to comment on this, according to the sponsor:

That could be a problem with blacklisted tokens, but we do not intend to do so according to the list of ERC20s posted

Sponsor agrees that the problem is associated with blacklisted tokens: and that is exactly what the function is doing based on the comment below

/**
     * @dev Allows the owner to recover other ERC20s mistakingly sent to this contract
     */

The function allows owner to recover other ERC20s that are mistakingly sent to this contract, if the ERC20 in question is USDC, then there will be a problem in case of blacklists. Mistakes can come in different magnitudes, This function could recover a large amount of USDC that could end up being locked in case of blacklists, I humbly request the issue to be accepted as a valid medium thank you!

[koolexcrypto]

Hi @pratokko

I understand your point.

The tokens listed are tokens in scope. Any other token that is not in scope, its behaviour not in scope as well.

There is no risk on the protocol of not recovering a token, since it was sent by mistake in the first place. It is not intended.