Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CEI violation in FxERC20ChildTunnel.sol #76

Open
b0gdaniy opened this issue Oct 10, 2024 · 0 comments
Open

CEI violation in FxERC20ChildTunnel.sol #76

b0gdaniy opened this issue Oct 10, 2024 · 0 comments

Comments

@b0gdaniy
Copy link

b0gdaniy commented Oct 10, 2024

There is a CEI (Checks-Effects-Interactions) pattern violation in FxERC20ChildTunnel.sol.
In costructor() assignment value tokenTemplate = _tokenTemplate; before _isContract(_tokenTemplate) check:

    constructor(address _fxChild, address _tokenTemplate) FxBaseChildTunnel(_fxChild) {
        tokenTemplate = _tokenTemplate; // Effect on state variable
        require(_isContract(_tokenTemplate), "Token template is not contract"); // Check
    }

so the code should look like this:

    constructor(address _fxChild, address _tokenTemplate) FxBaseChildTunnel(_fxChild) {
        require(_isContract(_tokenTemplate), "Token template is not contract"); // Check
        tokenTemplate = _tokenTemplate; // Interaction with state variable
    }

Also in _mapToken() function where
IFxERC20(childToken).initialize() called before rootToChildToken[rootToken] = childToken:

function _mapToken(bytes memory syncData) internal returns (address) {
        (address rootToken, string memory name, string memory symbol, uint8 decimals) = abi.decode(
            syncData,
            (address, string, string, uint8)
        );

        // get root to child token
        address childToken = rootToChildToken[rootToken];

        // check if it's already mapped
        require(childToken == address(0x0), "FxERC20ChildTunnel: ALREADY_MAPPED");

        // deploy new child token
        bytes32 salt = keccak256(abi.encodePacked(rootToken));
        childToken = createClone(salt, tokenTemplate);
        // slither-disable-next-line reentrancy-no-eth
        IFxERC20(childToken).initialize(
            address(this),
            rootToken,
            string(abi.encodePacked(name, SUFFIX_NAME)),
            string(abi.encodePacked(PREFIX_SYMBOL, symbol)),
            decimals
        ); // Interaction with childToken

        // map the token
        rootToChildToken[rootToken] = childToken; // Effect on state variable
        emit TokenMapped(rootToken, childToken);

        // return new child token
        return childToken;
    }

and code should look like:

function _mapToken(bytes memory syncData) internal returns (address) {
        (address rootToken, string memory name, string memory symbol, uint8 decimals) = abi.decode(
            syncData,
            (address, string, string, uint8)
        );

        // get root to child token
        address childToken = rootToChildToken[rootToken];

        // check if it's already mapped
        require(childToken == address(0x0), "FxERC20ChildTunnel: ALREADY_MAPPED");

        // deploy new child token
        bytes32 salt = keccak256(abi.encodePacked(rootToken));
        childToken = createClone(salt, tokenTemplate);

        // map the token
        rootToChildToken[rootToken] = childToken; // Effect on state variable
        
        // slither-disable-next-line reentrancy-no-eth
        IFxERC20(childToken).initialize(
            address(this),
            rootToken,
            string(abi.encodePacked(name, SUFFIX_NAME)),
            string(abi.encodePacked(PREFIX_SYMBOL, symbol)),
            decimals
        ); // Interaction with childToken
        
        emit TokenMapped(rootToken, childToken);

        // return new child token
        return childToken;
    }

These changes reduce gas cost if some of these checks will revert an error, before effects/interactions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant