tokamak-network / tokamak-thanos

MIT License
7 stars 3 forks source link

onlyEOA modifier does not prevent user sending ETH from a contract constructor #225

Closed mehdi-defiesta closed 3 weeks ago

mehdi-defiesta commented 3 weeks ago

Describe the bug in L1StandardBridge, users are allowed to send ETH to the contract to receive WETH on L2.

receive() external payable override onlyEOA {
    _initiateBridgeETH(msg.sender, msg.sender, msg.value, RECEIVE_DEFAULT_GAS_LIMIT, bytes(""));
}

onlyEOA modifier only checks that account.code.length == 0; Users can still bypass this modifier if they send ETH directly from the constructor which could be vulnerable to reentrancy.

Configuration

Impact user looping inside the receive function to call _initiateBridgeETH multiple time.

Recommendation adding conditions such as

require(tx.origin == msg.sender) 

Exploit Scenario

Demo

mehdi-defiesta commented 3 weeks ago

Update: In deposit flows, L1StandardBridge always call L1CrossDomainMessenger and L1CrossDomainMessenger always call OptimismPortal, so that there is no possible reentrancy attack in depositing.