Centralisation Risk: Admin Role of `TokenManagerEth` can Rug Pull All Eth from the Bridge

[code423n4]

Lines of code

https://github.com/skalenetwork/ima-c4-audit/blob/main/contracts/schain/TokenManagers/TokenManagerEth.sol#L45-L49

Vulnerability details

Impact

There is a Centralisation risk of the bridge where the DEFAULT_ADMIN_ROLE of TokenManagerEth.sol is able to modify the ERC20 token on the SChain to any arbitrary address. This would allow the admin role to change the address to one where they have infinite supply, they could then call exitToMain(amount) equal to the balance of the DepositBox in the main Ethereum chain. After the message is process on the main Ethereum chain they will receive the entire Eth balance of that contract.

The rug pull attack occurs because there is a DEFAULT_ADMIN_ROLE which is set in the intiialisation to the msg.sender as seen in initializeTokenManager() below.

The DEFAULT_ADMIN_ROLE may then call setEthErc20Address(IEthErc20 newEthErc20Address) setting newEthErc20Address to any arbitrary contract they control.

Proof of Concept

    function setEthErc20Address(IEthErc20 newEthErc20Address) external override {
        require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "Not authorized caller");
        require(ethErc20 != newEthErc20Address, "Must be new address");
        ethErc20 = newEthErc20Address;
    }

    function initializeTokenManager(
        string memory newSchainName,
        IMessageProxyForSchain newMessageProxy,
        ITokenManagerLinker newIMALinker,
        ICommunityLocker newCommunityLocker,
        address newDepositBox
    )
        public
        virtual
        initializer
    {
        require(newDepositBox != address(0), "DepositBox address has to be set");

        AccessControlEnumerableUpgradeable.__AccessControlEnumerable_init();
        _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
        _setupRole(AUTOMATIC_DEPLOY_ROLE, msg.sender);
        _setupRole(TOKEN_REGISTRAR_ROLE, msg.sender);

        schainHash = keccak256(abi.encodePacked(newSchainName));
        messageProxy = newMessageProxy;
        tokenManagerLinker = newIMALinker;
        communityLocker = newCommunityLocker;        
        depositBox = newDepositBox;

        emit DepositBoxWasChanged(address(0), newDepositBox);
    }

Recommended Mitigation Steps

Consider removing the function setEthErc20Address() as ethErc20 is set in the initialize() function and does not need to be changed.

[DimaStebaev]

This can be done only by SKALE chain owner. SKALE chain owner develops it's own application and have many other capabilities to spoof its users.

[GalloDaSballo]

Agree that the admin has the ability to rug users and agree with Med Severity