Beginner FriendlyFoundryBridge
100 EXP
View results
Submission Details
Severity: high
Valid

L1BossBridge contract is vulnerable to a signature replay attack, leading to all L1Vault's funds drained.

Summary

L1BossBridge contract has only one purpose : bridging an ERC20 token from the current layer (L1) to a L2. To achieve this, the contract can be called to deposit tokens through depositTokensToL2. This function will ultimately send tokens to L1Vault contract, and emit an event. This event is then listened by some L2 node and new tokens are minted on L2.

In order to withdraw tokens from L2 to L1, L1 bridge operator will listen to events from L2 and will generate a signature each time they receive a withdraw event from L2. They will sign a message with their private key, including the address of the ERC20 token, the value of eth to send with the call, and some data including events data (containing the actual function call to execute, i.e. token.transferFrom(address(vault), to, amount);.

Vulnerability Details

The vulnerability resides in the fact that the signed message doesn't contain any data uniquely matched to each call, such as a nonce variable. The signed message is :

abi.encode(
address(token),
0, // value
abi.encodeCall(IERC20.transferFrom, (address(vault), to, amount))
)
);

This means if the same user wants to withdraw again the same amount, the signature will be the same. This is problematic.
With the current implementation a valid signature could be used again and again by anyone, as blockchain transactions data are public.

Impact

This impact of this issue is HIGH as it could lead to the bridge being entirely drained. Anyone who executed one withdrawal from L2 to L1 could use a block explorer to retrieve the transaction's input data, and therefore the signature, and use it again and again, until L1Vault is empty.

Tools Used

Manual

Recommendations

I suggest to add a nonce variable into the signature. This nonce will be incremented after each withdrawal. This way, any previously valid signature will not be valid anymore because the nonce will be bigger, ultimately modifying the signature of the message.

The contract could declare a new storage variable :

public uint256 nonce;

and increase its value by one each time a withdrawal is executed.

withdrawTokensToL1 function could be modified as follows :

function withdrawTokensToL1(address to, uint256 amount, uint256 _nonce, uint8 v, bytes32 r, bytes32 s) external {
sendToL1(
v,
r,
s,
abi.encode(
address(token),
0, // value,
_nonce,
abi.encodeCall(IERC20.transferFrom, (address(vault), to, amount))
)
);
}

and sendToL1 function will be (except other issues declared in other findings):

function sendToL1(uint8 v, bytes32 r, bytes32 s, bytes memory message) public nonReentrant whenNotPaused {
address signer = ECDSA.recover(MessageHashUtils.toEthSignedMessageHash(keccak256(message)), v, r, s);
if (!signers[signer]) {
revert L1BossBridge__Unauthorized();
}
(address target, uint256 value, uint256 _nonce, bytes memory data) =
abi.decode(message, (address, uint256, uint256, bytes));
if (_nonce != nonce) {
revert L1BossBridge__IncorrectNonce();
}
(bool success,) = target.call{ value: value }(data);
if (!success) {
revert L1BossBridge__CallFailed();
}
nonce++;
}

after declaring a new custom error:

error L1BossBridge__IncorrectNonce();

This will ensure each successful withdrawal with sendToL1 function needs a new signature that includes a nonce value, incremented after each withdrawal. This will ultimately protect the bridge from signature replay attacks.

Nevertheless, this method is not perfect as it is now necessary to strictly respect order with :

  • Withdraw request on L2

  • Withdraw execution on L1 through withdrawTokensToL1 function

If there are many requests on L2 at the same time, users may encounter revert on their transaction if someone who requested before them didn't call withdrawTokensToL1 yet.

We could also imagine that L2 contract holds the nonce variable, and emits through the event this nonce value, incremented by one after each withdraw request. L1BossBridge contract could declare a mapping (uint256 -> bool) and check if the nonce used in the signature has already been used (i.e if the mapping value is set to true for this nonce value), updating the mapping after each successful withdrawal.

Updates

Lead Judging Commences

Hamiltonite Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

withdrawTokensToL1()/sendToL1(): signature replay

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.