sherlock-audit / 2024-08-flayer-judging

0 stars 0 forks source link

Winning Emerald Orca - Unprotected Initialization of ERC721 and ERC1155 Bridge Templates #714

Open sherlock-admin3 opened 4 days ago

sherlock-admin3 commented 4 days ago

Winning Emerald Orca

High

Unprotected Initialization of ERC721 and ERC1155 Bridge Templates

Summary

The InfernalRiftBelow contract contains two critical vulnerabilities in its initializeERC721Bridgable and initializeERC1155Bridgable functions. These functions lack access control, allowing any user to set the implementation addresses for ERC721 and ERC1155 bridgeable contracts. This can lead to unauthorized control over the bridge's core functionality and compromise the entire cross-chain NFT bridging mechanism.

Relevant Links

https://github.com/sherlock-audit/2024-08-flayer/blob/main/moongate/src/InfernalRiftBelow.sol#L103-L110 https://github.com/sherlock-audit/2024-08-flayer/blob/main/moongate/src/InfernalRiftBelow.sol#L119-L126

Vulnerability Detail

Both initializeERC721Bridgable and initializeERC1155Bridgable functions can be called by any external actor to set the respective implementation addresses. The only check in place prevents the addresses from being changed once set, but does not restrict who can initially set them.

    function initializeERC721Bridgable(address _erc721Bridgable) external {
    if (ERC721_BRIDGABLE_IMPLEMENTATION != address(0)) {
        revert TemplateAlreadySet();
    }

    ERC721_BRIDGABLE_IMPLEMENTATION = _erc721Bridgable;
    emit ERC721BridgableImplementationUpdated(_erc721Bridgable);
    }

    function initializeERC1155Bridgable(address _erc1155Bridgable) external {
        if (ERC1155_BRIDGABLE_IMPLEMENTATION != address(0)) { 
            revert TemplateAlreadySet();
        }

        ERC1155_BRIDGABLE_IMPLEMENTATION = _erc1155Bridgable;
        emit ERC1155BridgableImplementationUpdated(_erc1155Bridgable);
    }

Impact

An attacker can exploit these vulnerabilities by front-running the contract deployment transaction and setting the implementation addresses to contracts they control. This would give them complete control over the bridge's NFT handling mechanisms, potentially leading to:

Tool used

Manual Review

Recommendation

Implement proper access control for both initializeERC721Bridgable and initializeERC1155Bridgable functions. This can be achieved by:

Adding an owner or admin role to the contract or Using a modifier to restrict access to these functions.

Refactored Code

    function initializeERC721Bridgable(address _erc721Bridgable) external onlyOwner {
    if (ERC721_BRIDGABLE_IMPLEMENTATION != address(0)) {
        revert TemplateAlreadySet();
    }

    ERC721_BRIDGABLE_IMPLEMENTATION = _erc721Bridgable;
    emit ERC721BridgableImplementationUpdated(_erc721Bridgable);
    }

    function initializeERC1155Bridgable(address _erc1155Bridgable) external onlyOwner {
        if (ERC1155_BRIDGABLE_IMPLEMENTATION != address(0)) { 
            revert TemplateAlreadySet();
        }

        ERC1155_BRIDGABLE_IMPLEMENTATION = _erc1155Bridgable;
        emit ERC1155BridgableImplementationUpdated(_erc1155Bridgable);
    }