Missing receiver validation in withdraw functions.

[code423n4]

Lines of code

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

Vulnerability details

Impact

The withdraw functions does not validate its _to parameter. Funds can be lost if to is the zero address.

Similar issues have been judged as medium recently, see Sandclock M-15 / Github issue & Foundation M-09 / Github issue

Proof of Concept

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

/// @notice withdraw ETH in the case a held NFT earned ETH (ie. euler beats)
    function withdrawETH(address payable _to) external override {
        require(_isApprovedOrOwner(msg.sender, 0), "withdraw:not allowed");
        _to.transfer(address(this).balance);
        emit WithdrawETH(_to);
    }

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)));
    }
    ...
    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)));
        }
    }

Tools Used

Manual.

Recommended Mitigation Steps

Check that _to != address(0).

[mundhrakeshav]

call will be used instead of transfer

[HardlyDifficult]

Validating the input field is non-zero is a nice to have. It's effectively not much different than withdrawing and then using a standard transfer to address(0), or making a simple typo when inserting the expected address - which we cannot guard against in the contract. Lowing to Low risk and merging this with the warden's QA report https://github.com/code-423n4/2022-06-nibbl-findings/issues/297