code-423n4 / 2023-05-base-findings

1 stars 0 forks source link

Cross contract reentrancy attack through changing the xDomainMsgSender #100

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/ethereum-optimism/optimism/blob/382d38b7d45bcbf73cb5e1e3f28cbd45d24e8a59/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol#L393-L406

Vulnerability details

Impact

The use of the guaranteed safe CrossDomainMessenger for withdrawals can result in permanent blockages when the recipient address interacts with external addresses, which is an important feature for interoperability and is expected to be widely utilized.

Exploitation is possible for any withdrawals that interact with external addresses and then utilize the xDomainMessageSender() variable.

Proof of Concept

The CrossDomainMessenger (XDM) now allows re-entry as long as the message to be relayed is new. The following code snippet demonstrates how the message is delivered:

xDomainMsgSender = _sender;
bool success = SafeCall.callWithMinGas(_target, _minGasLimit, _value, _message);
xDomainMsgSender = Constants.DEFAULT_L2_SENDER;

The threat arises when a user's message is delivered, and the target contract calls another address for any reason. The called contract can perform a relayMessage on a previously failed transaction. It will reach the same code snippet mentioned above and, upon execution, set xDomainMsgSender = Constants.DEFAULT_L2_SENDER.

Upon returning to the original (victim) execution, this state variable is incorrect, as the correct sender should be _sender. Consequently, when the user's contract uses Optimism's API to obtain the sender, it will cause a revert.

function xDomainMessageSender() external view returns (address) {
    require(
        xDomainMsgSender != Constants.DEFAULT_L2_SENDER,
        "CrossDomainMessenger: xDomainMessageSender is not set"
    );
    return xDomainMsgSender;
}

It is highly likely that contracts receiving messages on L1 will (a) need to interact with untrusted addresses and (b) need to retrieve the L2 sender. These are the only conditions under which funds can become stuck.

These patterns are quite common, such as ERC721 safeTransfers triggering callbacks on the recipient and transferring funds with a call, which also involves control flow.

An example vulnerability is provided below, but any user-defined or widely used ecosystem contract could be affected:

function tipAndWithdrawRest(address tipped, uint256 amount) external payable {
    tipped.call{value:amount}("");
    uint256 leftovers = address(this).balance();
    ICrossDomainMessenger(cdm).xDomainMessageSender().call{value:leftovers}("");
}

While it can be argued that such transactions can then be replayed, the problematic flow resulting in a reverted transaction can be perpetuated indefinitely by the attacker. In the example above, the tipped address can be configured to perform reentrancy. Since the victim cannot modify the calldata for their withdrawal, they will encounter the same issue whenever they attempt to withdraw, causing their funds to become permanently stuck.

Tools Used

Manual Review

Recommended Mitigation Steps

One possible solution is to map the xDomainMsgSender based on the hash of the transaction. In other words, tracking the L2 sender address per message.

Assessed type

Reentrancy

code423n4 commented 1 year ago

Withdrawn by ladboy233