search

sherlock-audit / 2024-08-tokamak-network-judging

1 stars 0 forks source link

obront - All native token withdrawals to EOA will fail #43

Open sherlock-admin2 opened 3 weeks ago

sherlock-admin2 commented 3 weeks ago

obront

High

All native token withdrawals to EOA will fail

Summary

When a user withdraws the native token through the Cross Domain Messenger, the L1 result is that the token is approved, a callback is sent to the target address, and then the approval is removed. Since EOAs cannot act on a callback, the result is that all withdrawals of a native token to an EOA target will fail.

Root Cause

When native token is withdrawn through the Cross Domain Messenger, we send the tokens by (a) approving the token to the target address, (b) calling the target address with arbitrary calldata, and (c) revoking the approval. Here is the implementation:

if (_value != 0 && _target != address(0)) {
    IERC20(_nativeTokenAddress).approve(_target, _value);
}
bool success = SafeCall.call(_target, gasleft() - RELAY_RESERVED_GAS, 0, _message);
if (_value != 0 && _target != address(0)) {
    IERC20(_nativeTokenAddress).approve(_target, 0);
}

This pattern requires that the receiver is a contract with the ability to call safeTransfer() on the native token during that callback. This is of course not possible for EOAs, for whom the above call will be a no-op.

As issue similar to this seems to be mentioned in the Known Issues:

"Especially, in case of _tx.data.length is not 0 and _tx.data includes function relayMessage(uint256 _nonce, address _sender, address _target, uint256 _value, uint256 _minGasLimit, bytes calldata _message), user may lose funds even if _sender is EOA"

But that issue focuses on the _sender being an EOA.

This issue highlights the 100% losses that will occur in the event that the _target is an EOA.

Internal Preconditions

None

External Preconditions

None

Attack Path

  1. Any funds are sent to the L2 Cross Domain Messenger with _to as an L1 EOA address.

Impact

Anyone withdrawing a native token to an EOA wallet will lose their funds.

PoC

N/A

Mitigation

There are numerous possible solutions here. Some top contenders I'd recommend considering: 1) If the address is an EOA (or if some flag is set in the withdrawal), send the funds instead of approving them. 2) Keep the approvals set after the relayMessage() call, so the EOA can transfer them later (this would require using something like increaseAllowance() instead of approve()).