QA Report - Githubissues

[N-01] 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(,)

[N-02] Lock pragmas to specific compiler version Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively. see https://swcregistry.io/docs/SWC-103

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Utilities/AccessControlMechanism.sol#L4

Remediation Lock the pragma version and also consider known bugs (https://github.com/ethereum/solidity/releases) for the compiler version that is chosen. -pragma solidity ^0.8.0; +pragma solidity 0.8.12;

[N-03] Naming are not appropriate (1 letter) https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L557 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L558 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L559

[L-01] abi.encodePacked should not be used with dynamic types when passing the result to a hash function such as keccak256() https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVaultFactory.sol#L89-L91

bytes32 newsalt = keccak256(abi.encodePacked(_curator, _mix));
bytes memory code = abi.encodePacked(type(ProxyBasket).creationCode, uint256(uint160(basketImplementation)));
bytes32 hash = keccak256(abi.encodePacked(bytes1(0xff), address(this), newsalt, keccak256(code)));

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). “Unless there is a compelling reason, abi.encode should be preferred”. https://docs.soliditylang.org/en/v0.8.13/abi-spec.html#non-standard-packed-mode (in 'warning') as seen also in https://code4rena.com/reports/2022-04-backd (L-07)

[L-02] 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 https://github.com/code-423n4/2022-06-nibbl/blob/main/contracts/NibblVaultFactory.sol#L183

receive() payable external {    }

also in: https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L585 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Basket.sol#L114 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Proxy/ProxyBasket.sol#L56 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Proxy/ProxyVault.sol#L56

[L-03] Front-runable initializer If the initializer is not executed in the same transaction as the constructor, a malicious user can front-run the initialize() call, forcing the contract to be redeployed. This one appears not to be protected https://github.com/code-423n4/2022-06-nibbl/blob/main/contracts/NibblVault.sol#L173-L182

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

QA Report #311