sherlock-audit / 2022-10-mover-judging

1 stars 0 forks source link

dipp - chainId is not checked for Synapse bridging #55

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

dipp

medium

chainId is not checked for Synapse bridging

Summary

A lack of checks of the data passed to the Synapse bridge call could lead to funds being sent to the wrong chain.

Vulnerability Detail

In the _bridgeAssetDirect function in HardenedTopupProxy.sol the chain id given in _bridgeTxData in _bridgeAssetDirect is not checked to be 1 (L1 ETH mainnet) when the bridgeType is 0 (Synapse bridge) which could lead to funds being sent to the wrong chain.

Impact

User funds could be sent to the wrong chain.

Code Snippet

HardenedTopupProxy.sol:_bridgeAssetDirect#L416-L422

        if (_bridgeType == 0)
        {
            // Synapse bridge call data is retrieved by performing a call by the application
            // to bridge SDK and is not transformed by this contract
            bytes memory callData = _bridgeTxData.slice(20, _bridgeTxData.length - 20);
            (bool success, ) = targetAddress.call(callData);
            require(success, "BRIDGE_CALL_FAILED");

The Synapse bridge's deposit function which could be called with any chain id.

Synapse:L2BridgeZap:deposit#L250-L261

    function deposit(
        address to,
        uint256 chainId,
        IERC20 token,
        uint256 amount
    ) external {
        token.safeTransferFrom(msg.sender, address(this), amount);
        if (token.allowance(address(this), address(synapseBridge)) < amount) {
            token.safeApprove(address(synapseBridge), MAX_UINT256);
        }
        synapseBridge.deposit(to, chainId, token, amount);
    }

Tool used

Manual Review

Recommendation

Check in _bridgeTxData in the _bridgeAssetDirect function that the chain id passed to the Synapse bridge call is 1, for L1 ETH mainnet.

Duplicate of #49

McMannaman commented 2 years ago

Duplicate of https://github.com/sherlock-audit/2022-10-mover-judging/issues/49 We want keep some degree of flexibility by not hardcoding all parameters and expect application to send correct data to the smart contracts when user is not malicious and goes with the intended flow. If the user has intention to harm itself by formatting the incorrect call, well, he could do it.