code423n4 commented 2 years ago

Did not validate array length

Proof-of-Concept

The following function did not check if the _assetAddresses array and _assetIDs array are of the same length.

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L504

    ///@notice withdraw multiple ERC721s
    /// @param _assetAddresses the addresses of assets to be unlocked
    /// @param _assetIDs the IDs of assets to be unlocked
    /// @param _to the address where unlocked NFT will be sent
    function withdrawMultipleERC721(address[] memory _assetAddresses, uint256[] memory _assetIDs, address _to) external override boughtOut {
        require(msg.sender == bidder,"NibblVault: Only winner");
        for (uint256 i = 0; i < _assetAddresses.length; i++) {
            IERC721(_assetAddresses[i]).safeTransferFrom(address(this), _to, _assetIDs[i]);
        }
    }

Recommended Mitigation Steps

Implement check to ensure that _assetAddresses array and _assetIDs array are of the same length.

Lack of Re-entrancy Guard

Proof-of-Concept

When the safeTransferETH function is called, it is possible that the recipient can re-enter back to the function due to lack of re-entrancy guard.

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L398

function initiateBuyout() external override payable whenNotPaused returns(uint256 _buyoutBid) {
    require(block.timestamp >= minBuyoutTime, "NibblVault: minBuyoutTime < now");
    require(status == Status.initialized, "NibblVault: Status!=initialized");
    _buyoutBid = msg.value + (primaryReserveBalance - fictitiousPrimaryReserveBalance) + secondaryReserveBalance;
    //_buyoutBid: Bid User has made
    uint256 _currentValuation = getCurrentValuation();
    require(_buyoutBid >= _currentValuation, "NibblVault: Bid too low");
    // buyoutValuationDeposit = _currentValuation - ((primaryReserveBalance - fictitiousPrimaryReserveBalance) + secondaryReserveBalance); 
    buyoutValuationDeposit = msg.value - (_buyoutBid - _currentValuation);
    bidder = msg.sender;
    buyoutBid = _currentValuation;
    // buyoutBid: Bid can only be placed at current valuation
    buyoutRejectionValuation = (_currentValuation * (SCALE + REJECTION_PREMIUM)) / SCALE;
    buyoutEndTime = block.timestamp + BUYOUT_DURATION;
    status = Status.buyout;
    _updateTWAV(_currentValuation, uint32(block.timestamp % 2**32));
    if (_buyoutBid > _currentValuation) {
        safeTransferETH(payable(msg.sender), (_buyoutBid - _currentValuation));
    }
    emit BuyoutInitiated(msg.sender, _buyoutBid);
}

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L464

function redeem(address payable _to) external override boughtOut returns(uint256 _amtOut){
    uint256 _balance = balanceOf(msg.sender);
    _amtOut = ((address(this).balance - feeAccruedCurator - totalUnsettledBids) * _balance) / totalSupply();
    _burn(msg.sender, _balance);
    safeTransferETH(_to, _amtOut);
}