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

1 stars 0 forks source link

QA Report #239

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago
  1. Title : Used pause event for NibblVault

You can used this implementation for better use since it was no event and emitted inside contract

POC

https://docs.openzeppelin.com/contracts/2.x/api/lifecycle

Recommendation

Using event Paused(address account)

Emitted when the pause is triggered by a pauser (account).

event Unpaused(address account)

Emitted when the pause is lifted by a pauser (account).
  1. Title : Use whenPaused() modifier

Modifier to make a function callable only when the contract is paused.

POC

https://docs.openzeppelin.com/contracts/2.x/api/lifecycle#Pausable-whenPaused--

This module is used through inheritance. It will make available the modifiers whenNotPaused and whenPaused, which can be applied to the functions of your contract. Note that they will not be pausable by simply including this module, only once the modifiers are put in place.
  1. Title : Standart use of function initialize() was used so many times. So if necessary it can be changed from basket interface and basket.sol into intialize , you can removed another initialise and changed into initialize. Since it was good for readibility and structure.

Tool Used

Manual Review

  1. Title : Avoid multiple initializations

https://github.com/NibblNFT/nibbl-smartcontracts/blob/49bf364d9e81a554cfdf47ae5cfc3daf52a54ad6/contracts/NibblVault.sol#L173

Make sure though that you do not allow multiple initializations. For just a few parameters, simply add a check for each parameter, For many parameters, add an isInitialized boolean state variable:

contract MyContract {
  bool isInitialized = false;

  function initialize(
    uint256 _param1,
    uint256 _param2,  
    uint256 _param3,
    address _param4,
    address _param5,
    bytes32 _param6,
    bytes32 _param7
  ) public {
    require(!isInitialized, 'Contract is already initialized!');
    isInitialized = true;

    param1 = _param1;
    ...
    param7 = _param7;
  }
}

Tool Used

Manual Review

  1. Title : Avoid Floatin Pragma in AccessControlMechanism.sol

https://github.com/code-423n4/2022-06-nibbl/blob/main/contracts/Utilities/AccessControlMechanism.sol

Since it was used ^0.8.0. As the compiler can be use as 0.8.10 and consider locking at this version the same as another. It can be consider using locking the pragma version whenever possible and avoid using a floating pragma in the final deployment. Since it can be problematic, if there are publicly disclosed bugs and issues that affect the current compiler version used.

Tool Used

Manual Review

HardlyDifficult commented 2 years ago

4 is invalid. Others are NC