Nibbl contest - Randyyy's results

Lines of code

https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L517

Vulnerability details

Vulnerability Details

the withdrawERC20 function use the standard IERC20 interface from openzeppelin, which expect a returns from the interface, this works for the most ERC20 token, however for USDT token that didnt follow the ERC20 standard didnt have a return value on the transfer function, if the basket contract hold any USDT token, the USDT token would be stuck because when the user call withdrawERC20 with asset set as USDT, this transaction would revert and the token is not transferred.

Mitigation

consider using safeERC20 from openzepelin, to handle odd ERC20 token

Impact

Permanent freezing for USDT token.

POC

https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L517

1. SafeMath is not necressary

Impact

The Swap contract uses Solidity version 0.8 which already implements overflow checks by default. At the same time, it uses the SafeMath library which is more gas expensive than the 0.8 overflow checks.

POC

https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVaultFactory.sol#L9

2. bloktimesamp can be simpler and save gas.

Impact

Use uint32(n) can cut off higher bit already.

POC

https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L303 https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L366

Navigation

uint32 blockTimestamp = uint32(block.timestamp);

3. Use Custom Errors

Impact

Custom errors from solidity 0.8.4 are cheaper than revert strings

POC

https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L129 https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L139 https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L146 https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L147

4. uint default value is zero

Impact

uint256 variable are initialized to a default value of zero. setting variable to the default value is unnecessary.

POC

https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L506 https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L525

5. Cached array length can save gas.

Impact

Caching array length in the stack save gas per iteration.

POC

https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L506 https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L525 https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVault.sol#L547

6. Unnecessary MSTORE.

Impact

Instead of wraping _basketAddress with Basket type to call initialise, u can do this without an extra MSTORE by changing this

function createBasket(address _curator, string memory _mix) public override returns(address)  {
        address payable _basketAddress = payable(new ProxyBasket{salt: keccak256(abi.encodePacked(_curator, _mix))}(basketImplementation));
        Basket _basket = Basket(_basketAddress);
        _basket.initialise(_curator);
        emit BasketCreated(_curator, _basketAddress);
        return _basketAddress;
    }

to this

function createBasket(address _curator, string memory _mix) public override returns(address)  {
        address payable _basketAddress = payable(new ProxyBasket{salt: keccak256(abi.encodePacked(_curator, _mix))}(basketImplementation));
        Basket(_basketAddress).initialise(_curator);
        emit BasketCreated(_curator, _basketAddress);
        return _basketAddress;
    }

POC

https://github.com/code-423n4/2022-06-nibbl/blob/bcae7d71718c8eb744e4465d68fd6efa2bde7b06/contracts/NibblVaultFactory.sol#L82