First Flight #4: Boss Bridge

[L-01] Missing checks for address(0) in constructor/initializers

Severity

• Impact: Medium

• Likelihood: Low

Description

In Solidity, the Ethereum address 0x0000000000000000000000000000000000000000 is known as the "zero address". This address has significance because it's the default value for uninitialized address variables and is often used to represent an invalid or non-existent address. The "Missing zero address control" issue arises when a Solidity smart contract does not properly check or prevent interactions with the zero address, leading to unintended behavior.
For instance, a contract might allow tokens to be sent to the zero address without any checks, which essentially burns those tokens as they become irretrievable. While sometimes this is intentional, without proper control or checks, accidental transfers could occur.

Number Of Instances Found

2

Code Location

Click to show findings

Path: ./src/L1Vault.sol

​

16: token = _token; // @audit-issue

GitHub: 16

Path: ./src/L1BossBridge.sol

​

43: token = _token; // @audit-issue

GitHub: 43

[L-02] Use Ownable2Step rather than Ownable

Severity

• Impact: Low

• Likelihood: Medium

Description

Ownable2Step and Ownable2StepUpgradeable prevent the contract ownership from mistakenly being transferred to an address that cannot handle it (e.g. due to a typo in the address), by requiring that the recipient of the owner permissions actively accept via a contract call of its own.

Number Of Instances Found

3

Code Location

Click to show findings

Path: ./src/L1Vault.sol

​

12:contract L1Vault is Ownable { // @audit-issue

GitHub: 12

Path: ./src/TokenFactory.sol

​

11:contract TokenFactory is Ownable { // @audit-issue

GitHub: 11

Path: ./src/L1BossBridge.sol

​

27:contract L1BossBridge is Ownable, Pausable, ReentrancyGuard { // @audit-issue

GitHub: 27

[L-03] Return values of approve not checked with IERC20

Severity

• Impact: Medium

• Likelihood: Low

Description

Not all IERC20 implementations revert when there's a failure in approve. The function signature has a boolean return value and they indicate errors that way instead.

By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything. self.impact_details =

Number Of Instances Found

1

Code Location

Click to show findings

Path: ./src/L1Vault.sol

​

20: token.approve(target, amount); // @audit-issue

GitHub: 20

[L-04] Large transfers may not work with some ERC20 tokens

Severity

• Impact: Medium

• Likelihood: Low

Description

Not all IERC20 implementations are totally compliant, and some (e.g UNI, COMP) may fail if the valued passed is larger than uint96. Source

Number Of Instances Found

1

Code Location

Click to show findings

Path: ./src/L1BossBridge.sol

​

74: token.safeTransferFrom(from, address(vault), amount); // @audit-issue

GitHub: 74

[L-05] Large approvals may not work with some ERC20 tokens

Severity

• Impact: Medium

• Likelihood: Low

Description

Not all IERC20 implementations are totally compliant, and some (e.g UNI, COMP) may fail if the valued passed is larger than uint96. Source

Number Of Instances Found

1

Code Location

Click to show findings

Path: ./src/L1Vault.sol

​

20: token.approve(target, amount); // @audit-issue

GitHub: 20

[L-06] approve can revert if the current approval is not zero

Severity

• Impact: Medium

• Likelihood: Low

Description

Some tokens like USDT check for the current approval, and they revert if it's not zero. While Tether is known to do this, it applies to other tokens as well, which are trying to protect against this attack vector.

Number Of Instances Found

1

Code Location

Click to show findings

Path: ./src/L1Vault.sol

​

20: token.approve(target, amount); // @audit-issue

GitHub: 20

[L-07] Functions calling contracts/addresses with transfer hooks are missing reentrancy guards

Severity

• Impact: Medium

• Likelihood: Low

Description

Even if the function follows the best practice of check-effects-interaction, not using a reentrancy guard when there may be transfer hooks will open the users of this protocol up to read-only reentrancies with no way to protect against it, except by block-listing the whole protocol.

Number Of Instances Found

1

Code Location

Click to show findings

Path: ./src/L1BossBridge.sol

​

74: token.safeTransferFrom(from, address(vault), amount); // @audit-issue

GitHub: 74