code-423n4 / 2022-06-nested-findings

0 stars 1 forks source link

QA Report #30

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago
  1. Contract locking ether The following contracts have a payable function but has no mechanism to withdraw the eth in the contract. OwnerProxy.sol FlatOperator.sol

Tool Used: Slither

  1. missing zero address check The following are missing checks for existence of zero address which may lead to redeployment of contract or function reverting.

**Occurrences in:

  1. Unnecessary import IFlatOperator does not need to import ERC20 since the transfer() function is custom defined and does not conform to ERC20's transfer.

  2. Costly External calls inside a loop **Occurrences in:

https://github.com/code-423n4/2022-06-nested/blob/main/contracts/operators/Beefy/BeefyVaultOperator.sol#L19 https://github.com/code-423n4/2022-06-nested/blob/main/contracts/operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol#L28 *https://github.com/code-423n4/2022-06-nested/blob/main/contracts/operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol#L28

obatirou commented 2 years ago

4. Costly External calls inside a loop (disputed)

We want to loop through the array to whitelist some vault/strategy at the deployment As long as this process is in the constructor, those vaults / strategy will be reachable after the deployment

obatirou commented 2 years ago

1. Contract locking ether (disputed)

FlatOperator is not supposed to be called outside of a delegateCall context

For OwnerProxy we can set the value to zero to not send ether. And if we made a mistake, we can create a script to send the ether back.

Yashiru commented 2 years ago

2. missing zero address check (Duplicated)

Duplicated of #61 at 2. Missing address(0) checks