First Flight #4: Boss Bridge

Summary

The function depositTokensToL2 transfers tokens from one address to another and emits an event, but does not follow the check-effects-interactions pattern.

Vulnerability Details

Reentrancy in L1BossBridge.depositTokensToL2(address,address,uint256) (src/L1BossBridge.sol#70-78):
External calls:

• token.safeTransferFrom(from,address(vault),amount) (src/L1BossBridge.sol#74)
  Event emitted after the call(s):

• Deposit(from,l2Recipient,amount) (src/L1BossBridge.sol#77)

Impact

The impact of this issue is that the function depositTokensToL2 could be abused by an attacker who can create a malicious contract that implements the token interface and overrides the safeTransferFrom method. The attacker could then call the function depositTokensToL2 with their own address as the from parameter, and trigger the safeTransferFrom method of their contract. This method could then call back the function depositTokensToL2 recursively, and emit multiple Deposit events with different values or recipients. This could result in a discrepancy between the tokens transferred on L1 and the tokens minted on L2, and cause a loss of funds or a breach of the security and integrity of the system. Therefore, it is important to prevent reentrancy attacks by following the check-effects-interactions pattern.

Tools Used

• slither

Recommendations

you should apply the check-effects-interactions pattern, which is a design principle that suggests to perform all checks and state changes before calling external contracts or sending Ether.
Apply the check-effects-interactions pattern.