Platform: Code4rena
Start Date: 21/06/2022
Pot Size: $30,000 USDC
Total HM: 12
Participants: 96
Period: 3 days
Judge: HardlyDifficult
Total Solo HM: 5
Id: 140
League: ETH
Rank: 46/96
Findings: 2
Award: $45.67
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x29A, 0x52, 0xNazgul, 0xNineDec, 0xc0ffEE, 0xf15ers, 0xkatana, BowTiedWardens, Chom, ElKu, Funen, GalloDaSballo, JC, JMukesh, JohnSmith, Lambda, Limbooo, MadWookie, MiloTruck, Nethermind, Noah3o6, Nyamcil, Picodes, PwnedNoMore, Randyyy, RoiEvenHaim, SmartSek, StErMi, Tadashi, TerrierLover, TomJ, Tomio, Treasure-Seeker, UnusualTurtle, Varun_Verma, Wayne, Waze, _Adam, apostle0x01, asutorufos, berndartmueller, c3phas, catchup, cccz, cloudjunky, codexploder, cryptphi, defsec, delfin454000, dipp, ellahi, exd0tpy, fatherOfBlocks, hansfriese, hyh, joestakey, kebabsec, kenta, masterchief, minhquanym, naps62, oyc_109, pashov, peritoflores, reassor, rfa, robee, sach1r0, saian, sashik_eth, shenwilly, simon135, slywaters, sorrynotsorry, sseefried, unforgiven, xiaoming90, ych18, zuhaibmohd, zzzitron
28.3011 USDC - $28.30
Some ERC20 tokens may return false instead of reverting when transfer fails, so transaction will succeed even when token transfer fails
IERC20(_asset).transfer(_to, IERC20(_asset).balanceOf(address(this)));
IERC20(_assets[i]).transfer(_to, IERC20(_assets[i]).balanceOf(address(this)));
IERC20(_tokens[i]).transfer(msg.sender, IERC20(_tokens[i]).balanceOf(address(this)));
IERC20(_tokens[i]).transfer(msg.sender, IERC20(_tokens[i]).balanceOf(address(this)));
Add a require statement to check the return value, or use Openzeppelin's SafeERC20
transfer method forwards a fixed amount of gas 2300, which may cause reverts if the payable callback consumes more than the fixed gas or if gas costs changes in the future
https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/
_to.transfer(address(this).balance);
transfer can be replaced with sendValue which uses call method
Values passed into the functions are not validated which may lead to accidentally setting variables to wrong values
function updateCurator(address _newCurator) external override { require(msg.sender == curator,"NibblVault: Only Curator"); curator = _newCurator; }
Add input validation statements
Event indexed parameters are stored in the topics part of the log which allows for quicker querying by off-chain monitoring tools and scripts,
function createVault(address _assetAddress, address _curator, string memory _name, string memory _symbol, uint256 _assetTokenID, uint256 _initialSupply, uint256 _initialTokenPrice, uint256 _minBuyoutTime) external payable returns(address payable _proxyVault);
Events that change critical parameters should emit an event which helps users to easily monitor changes in the system
Some functions are missing natspec comments
function getVaults() public view returns(ProxyVault[] memory ) {
function createBasket(address _curator, string memory _mix) public override returns(address) {
function getBasketAddress(address _curator, string memory _mix) public override view returns(address _basket) {
function withdrawMultipleERC721(address[] memory _tokens, uint256[] memory _tokenId, address _to) external override
function withdrawMultipleERC1155(address[] memory _tokens, uint256[] memory _tokenIds, address _to) external override
Missing @param comment
/// @notice Function for tokenholders to redeem their tokens for reserve token in case of buyout success /// @dev The redeemed reserve token are in proportion to the token supply someone owns /// @dev The amount available for redemption is contract balance - (total unsettled bid and curator fees accrued) function redeem(address payable _to) external override boughtOut returns(uint256 _amtOut)
/// @notice withdraw an ERC721 token from this contract into your wallet /// @param _token the address of the NFT you are withdrawing /// @param _tokenId the ID of the NFT you are withdrawing function withdrawERC721Unsafe(address _token, uint256 _tokenId, address _to) external override {
/// @notice withdraw an ERC721 token from this contract into your wallet /// @param _token the address of the NFT you are withdrawing /// @param _tokenId the ID of the NFT you are withdrawing function withdrawERC1155(address _token, uint256 _tokenId, address _to) external override
/// @notice withdraw ETH in the case a held NFT earned ETH (ie. euler beats) function withdrawETH(address payable _to) external override
/// @notice withdraw ERC20 in the case a held NFT earned ERC20 function withdrawERC20(address _token) external override
Remove commented out code
createBasket missing whenNotPaused modifierFunction createBasket is missing the whenNotPaused modifier, so basket can be created when the contract is paused
function createBasket(address _curator, string memory _mix) public override returns(address) {
Add whenNotPaused modifier to createBasket function
public functions which are not accessed inside the contracts can be converted to external
contracts/NibblVaultFactory.sol
function getVaultAddress( address _curator, address _assetAddress, uint256 _assetTokenID, uint256 _initialSupply, uint256 _initialTokenPrice) public view returns(address _vault) function getVaults() public view returns(ProxyVault[] memory ) function createBasket(address _curator, string memory _mix) public override returns(address) function getBasketAddress(address _curator, string memory _mix) public override view returns(address _basket) {
function withdrawMultipleERC1155(address[] memory _assets, uint256[] memory _assetIDs, address _to)
require statement to check array lengths can be added
Files imported are not used
#0 - HardlyDifficult
2022-07-04T19:03:03Z
Good feedback
🌟 Selected for report: IllIllI
Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 8olidity, ACai, BowTiedWardens, Chandr, Chom, ElKu, Fitraldys, Funen, IgnacioB, JC, Lambda, Limbooo, MiloTruck, Noah3o6, Nyamcil, Picodes, Randyyy, SmartSek, StErMi, TerrierLover, TomJ, Tomio, UnusualTurtle, Waze, _Adam, ajtra, c3phas, cRat1st0s, catchup, codexploder, cryptphi, defsec, delfin454000, ellahi, exd0tpy, fatherOfBlocks, hansfriese, joestakey, kebabsec, kenta, m_Rassska, minhquanym, oyc_109, pashov, reassor, rfa, robee, sach1r0, saian, sashik_eth, simon135, slywaters, ych18, ynnad, zuhaibmohd
17.3709 USDC - $17.37
Revert strings longer than 32 bytes will increase deployment gas as well as runtime gas when revert condition is met
require(msg.value >= MIN_INITIAL_RESERVE_BALANCE, "NibblVaultFactory: Initial reserve balance too low");
require(IERC721(_assetAddress).ownerOf(_assetTokenID) == msg.sender, "NibblVaultFactory: Invalid sender");
require(basketUpdateTime != 0 && block.timestamp >= basketUpdateTime, "NibblVaultFactory: UPDATE_TIME has not passed");
require(feeToUpdateTime != 0 && block.timestamp >= feeToUpdateTime, "NibblVaultFactory: UPDATE_TIME has not passed");
require(_newFee <= MAX_ADMIN_FEE, "NibblVaultFactory: Fee value greater than MAX_ADMIN_FEE");
require(feeAdminUpdateTime != 0 && block.timestamp >= feeAdminUpdateTime, "NibblVaultFactory: UPDATE_TIME has not passed");
require(vaultUpdateTime != 0 && block.timestamp >= vaultUpdateTime, "NibblVaultFactory: UPDATE_TIME has not passed");
require(buyoutEndTime <= block.timestamp, "NibblVault: buyoutEndTime <= now");
require(_secondaryReserveRatio >= MIN_SECONDARY_RESERVE_RATIO, "NibblVault: secResRatio too low");
Reduce the size of strings to 32 bytes or use custom errors
Function parameters that are read-only can be converted to calldata to avoid it being copied to memory which will consume more gas
function createVault( address _assetAddress, address _curator, string memory _name, string memory _symbol, uint256 _assetTokenID, uint256 _initialSupply, uint256 _initialTokenPrice, uint256 _minBuyoutTime ) external payable override whenNotPaused returns(address payable _proxyVault)
function createBasket(address _curator, string memory _mix) public override returns(address)
function getBasketAddress(address _curator, string memory _mix) public override view returns(address _basket)
function initialize( string memory _tokenName, string memory _tokenSymbol, address _assetAddress, uint256 _assetID, address _curator, uint256 _initialTokenSupply, uint256 _initialTokenPrice, uint256 _minBuyoutTime ) external override initializer payable
Initialising variables with default values is not necessary as variables already contains default values, array length can be cached in a temporary variable and reused in the for loops instead of querying length in each iteration post-increment can be replaced with pre-increment as pre-increment consumes less gas, and unchecked can be added to avoid the default overflow/underflow checks
unchecked to save gasAdding unchecked to expression that dont overflow can avoid the default overflow/underflow checks introduced in 0.8 and save gas
https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L319
_purchaseReturn = _initialTokenSupply - _totalSupply;
https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L378
uint256 _tokensPrimaryCurve = _totalSupply - _initialTokenSupply;