code423n4 commented 2 years ago

Low

[L-01] Unsafe ERC20 Operations

Impact

The return value of an external transfer/transferFrom call is not checked

Proof of Concept

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Basket.sol#L84-L98

   /// @notice withdraw ERC20 in the case a held NFT earned ERC20
    function withdrawERC20(address _token) external override {
        require(_isApprovedOrOwner(msg.sender, 0), "withdraw:not allowed");
        IERC20(_token).transfer(msg.sender, IERC20(_token).balanceOf(address(this)));
        emit WithdrawERC20(_token, msg.sender);
    }

    function withdrawMultipleERC20(address[] memory _tokens) external override {
        require(_isApprovedOrOwner(msg.sender, 0), "withdraw:not allowed");
        for (uint256 i = 0; i < _tokens.length; i++) {
            IERC20(_tokens[i]).transfer(msg.sender, IERC20(_tokens[i]).balanceOf(address(this)));
            emit WithdrawERC20(_tokens[i], msg.sender);
        }
    }

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

function withdrawERC20(address _asset, address _to) external override boughtOut {
        require(msg.sender == bidder, "NibblVault: Only winner");
        IERC20(_asset).transfer(_to, IERC20(_asset).balanceOf(address(this)));
    }

    /// @notice withdraw multiple ERC20s
    /// @param _assets the addresses of assets to be unlocked
    /// @param _to the address where unlocked NFTs will be sent
    function withdrawMultipleERC20(address[] memory _assets, address _to) external override boughtOut {
        require(msg.sender == bidder, "NibblVault: Only winner");
        for (uint256 i = 0; i < _assets.length; i++) {
            IERC20(_assets[i]).transfer(_to, IERC20(_assets[i]).balanceOf(address(this)));
        }
    }