sherlock-audit / 2024-02-telcoin-platform-audit-update-judging

3 stars 1 forks source link

cats - Bridge Relay unable to transfer some popular `ERC-20` tokens #6

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

cats

high

Bridge Relay unable to transfer some popular ERC-20 tokens

Summary

The Bridge Relay contract is not able to transfer ERC-20 tokens.

Vulnerability Detail

If a user wants to bridge an ERC-20 token in the BridgeRelay.sol contract, the following function is called:

    function bridgeTransfer(IERC20 token) external payable {
        // revert if MATIC is attempted
        if (token == MATIC) revert MATICUnbridgeable();
        // unwrap WETH
        if (token == WETH) {
            IERC20Withdrawable(address(WETH)).withdraw(
                WETH.balanceOf(address(this))
            );
            // transfer ERC20 tokens
@>      } else if (token != ETHER) {
@>          transferERCToBridge(token);
            return;
        }
        // transfer ETHER
        POS_BRIDGE.depositEtherFor{value: address(this).balance}(address(this));
    }

The issue is that if we open up the POS_BRIDGE proxy contract on etherscan and query for example the MNT Token address in the rootToChildToken mapping, we will see that the mapping returns address(0). Due to this, the depositFor function will revert at line #2206 of the bridge implementation contract:

    function _depositFor(
        address user,
        address rootToken,
        bytes memory depositData
    ) private {
        bytes32 tokenType = tokenToType[rootToken];
        require(
@>          rootToChildToken[rootToken] != address(0x0) &&
               tokenType != 0,
            "RootChainManager: TOKEN_NOT_MAPPED"
        );
    }

Two other tokens that will not work are WLD and KCS. Given that the contest README expects all ERC-20 to interact with the contracts, tokens deemed top 30 in market cap should be able to be used. Also, the erc20Rescue function can only rescue stuck MATIC, so all tokens that fail to be bridged will be stuck in the contract.

Impact

Cannot bridge popular tokens when the protocol should function with them. Will lead to token stuck in contract forever since erc20Rescue only rescues MATIC.

Code Snippet

    function bridgeTransfer(IERC20 token) external payable {
        // revert if MATIC is attempted
        if (token == MATIC) revert MATICUnbridgeable();
        // unwrap WETH
        if (token == WETH) {
            IERC20Withdrawable(address(WETH)).withdraw(
                WETH.balanceOf(address(this))
            );
            // transfer ERC20 tokens
        } else if (token != ETHER) {
            transferERCToBridge(token);
            return;
        }
        // transfer ETHER
        POS_BRIDGE.depositEtherFor{value: address(this).balance}(address(this));
    }

Tool used

Manual Review

Recommendation

Set a whitelist of allowed tokens.

sherlock-admin3 commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

WangAudit commented:

user mistake to bridge tokens that are not on polygon