sherlock-audit / 2023-05-ecoprotocol-judging

1 stars 0 forks source link

SanketKogekar - Missing `onlyEOA` modifier on `depositERC20To` function. #124

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

SanketKogekar

medium

Missing onlyEOA modifier on depositERC20To function.

Summary

There is no onlyEOA modifier on depositERC20To function, but it is on depositERC20

Vulnerability Detail

The comments says "Used to stop deposits from contracts (avoid accidentally lost tokens)" and uses it for depositERC20 fuction.

/**
     * @dev Modifier requiring sender to be EOA.  This check could be bypassed by a malicious
     * contract via initcode, but it takes care of the user error we want to avoid.
     */
    modifier onlyEOA() {
        // Used to stop deposits from contracts (avoid accidentally lost tokens)
        require(msg.sender.code.length == 0, "L1ECOBridge: Account not EOA");
        _;
    }

The only change depositERC20To is that it deposits tokens to specified address. So it still needs to "stop deposits from contracts (avoid accidentally lost tokens)" as specified.

Impact

Not using onlyEOA won't stop deposits from contracts.

Code Snippet

https://github.com/sherlock-audit/2023-05-ecoprotocol/blob/b440f89234b806f672b9e9ad24cf70c409964db5/op-eco/contracts/bridge/L1ECOBridge.sol#L219

Tool used

Manual Review

Recommendation

function depositERC20To(
        address _l1Token,
        address _l2Token,
        address _to,
        uint256 _amount,
        uint32 _l2Gas,
        bytes calldata _data
    ) external virtual onlyEOA isL1EcoToken(_l1Token) isL2EcoToken(_l2Token) {
...
}

Duplicate of #72