obront - `onApprove()` can force accidental contract creation

[sherlock-admin2]

obront

Medium

onApprove() can force accidental contract creation

Summary

Deposit transactions in the portal allow the user to specify whether or not the intention is contract creation on L2. The onApprove() function makes the assumption that all transactions to address(0) are intended to be contract creations, and can thus redirect otherwise valid transactions into failed contract creations.

Root Cause

When depositTransaction() is called on the Optimism Portal, two of the values passed are to and isCreation.

function depositTransaction(
    address _to,
    uint256 _mint,
    uint256 _value,
    uint64 _gasLimit,
    bool _isCreation,
    bytes calldata _data
)

These two values have the following relationship: 1) IF _isCreation is true, then _to must be address(0). 2) IF _to is address(0), _isCreation can be either true or false.

As we can see in the op-node code here, in the event that _isCreation = true, we set the address to nil, which causes a contract to be created. Otherwise, we leave the address as address(0) and perform the transaction as normal.

While it is unlikely that a user would want to send a transaction to address(0), it is possible, which is why the above flow exists. These checks and requirements are the exact same in the Tokamak version of the contracts.

However, there is another way that the Tokamak contract allows a user to initiate a deposit transaction, and that is with the approveAndCall() from the native token. In this case, we can see the logic below:

function onApprove(
    address _owner, // msg.sender
    address, // spender
    uint256 _amount, // amount
    bytes calldata _data // anything
)
    external
    override
    returns (bool)
{
    (address to, uint256 value, uint32 gasLimit, bytes calldata message) = unpackOnApproveData(_data);
    if (msg.sender == _nativeToken()) {
        _depositTransaction(_owner, to, _amount, value, gasLimit, to == address(0), message, true);
        return true;
    } else {
        return false;
    }
}

As we can see, in this case _isCreation is automatically set to true when to == address(0).

Internal Preconditions

None

External Preconditions

None

Attack Path

1. A user wants to send an L2 transaction to address(0).
2. They use the approveAndCall() function on the native token to send the transaction.
3. Their L2 transaction is turned into a contract creation, which wasn't intended.

Impact

L2 transactions can occur differently than expected, breaking user expectations.

PoC

N/A

Mitigation

The onApprove() data should be updated to include the _isCreation flag, so that the user can specify whether or not the transaction is intended to be a contract creation.