search

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

1 stars 0 forks source link

QA Report #296

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1. NATSPEC IS INCOMPLETE

Examples of this issue in the codebase:

  1. File: contracts\NibblVaultFactory.sol missing @return (Line 38)

  2. File: contracts\NibblVaultFactory.sol missing @return (Line 64)

  3. File: contracts\NibblVaultFactory.sol missing @return @param _curator @param _mix (Line 80)

2. constants should be defined rather than using magic numbers

Examples of this issue in the codebase:

  1. File: contracts\NibblVault.sol (Line 176)

  2. File: contracts\NibblVault.sol (Line 183)

3. Missing checks for address(0x0) when assigning values to address state variables

Examples of this issue in the codebase:

  1. File: contracts\NibblVault.sol (Line 191)

  2. File: contracts\NibblVault.sol (Line 193)

4. Use a more recent version of solidity

Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(,) Examples of this issue in the codebase:

  1. File: contracts\NibblVaultFactory.sol (Line 3)

5. abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Examples of this issue in the codebase:

  1. File: contracts\NibblVaultFactory.sol (Line 70)

  2. File: contracts\NibblVaultFactory.sol (Line 91)

6. Unused receive() function will lock Ether in contract

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert Examples of this issue in the codebase:

  1. File: contracts\NibblVaultFactory.sol (Line 183)

  2. File: contracts\NibblVault.sol (Line 585)

6. Unused receive() function will lock Ether in contract

If the intention is for the Ether to be used, the function should call another function, otherwise, it should revert Examples of this issue in the codebase:

  1. File: contracts\NibblVaultFactory.sol (Line 183)

  2. File: contracts\NibblVault.sol (Line 585)

7. Constant redefined elsewhere

Consider defining in only one contract so that values cannot become out of sync when only one location is updated. If the variable is a local cache of another contract's value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don't get out of sync.

Example of this issue in the codebase: factory is seen in File: contracts\NibblVault.sol (Line 60) and File: contracts\ProxyFault.sol (Line 17)

HardlyDifficult commented 2 years ago

All valid considerations