sherlock-audit / 2023-03-optimism-judging

6 stars 0 forks source link

chaduke - OptimismPortal#receive() failes to make sure msg.sender is not a contract #18

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

chaduke

medium

OptimismPortal#receive() failes to make sure msg.sender is not a contract

Summary

It is is important for OptimismPortal#receive() to make sure the caller is not a contract. Otherwise, any deposited funds will be lost due to address aliasing.

Vulnerability Detail.

Receive() accepts value so that users can send ETH directly to this contract and have the funds be deposited to their address on L2.

https://github.com/ethereum-optimism/optimism/blob/9b9f78c6613c6ee53b93ca43c71bb74479f4b975/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol#L197-L199

However, it does not check whether msg.sender is a contract or not. When msg.sender is a contract, any deposited funds will be lost due to address aliasing.

Impact

OptimismPortal#receive() does not check and make sure the caller is not a contract. As a reuslt, any deposited funds might be lost due to address aliasing.

Code Snippet

Tool used

VSCode

Manual Review

Recommendation

Check msg.sender is EOA only:

receive() external payable {
+        if(msg.sender != tx.origin) revert NotEOA();
        depositTransaction(msg.sender, msg.value, RECEIVE_DEFAULT_GAS_LIMIT, false, bytes(""));
    }
GalloDaSballo commented 1 year ago

QA because QA in previous contest

hrishibhat commented 1 year ago

Sponsor comment: This is a low actionable issue.