withdraw of assets batch could be blocked with one asset with malicious code

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L466-L520 https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L636-L661

Vulnerability details

Impact

withdraw() function calls internal functions that tranfers assets - _transferERC20sOut, _transferERC721sOut, _transferFloorsOut. They could only run through all transferring assets in one cycle. If one of transferring assets has malicious code that blocks all transfers, except those allowed by an attacker, then an attacker could demand a ransom from the victim for allowing the withdrawal of blocked assets.

    /**
        @notice Transfers an array of erc20 tokens to the msg.sender.
        @param assets The erc20 tokens and amounts to send.
     */
    function _transferERC20sOut(ERC20Asset[] memory assets) internal {
        for (uint256 i = 0; i < assets.length; i++) {
            ERC20(assets[i].token).safeTransfer(msg.sender, assets[i].tokenAmount);
        }
    }

    /**
        @notice Transfers an array of erc721 tokens to the msg.sender.
        @param assets The erc721 tokens and token ids to send.
     */
    function _transferERC721sOut(ERC721Asset[] memory assets) internal {
        for (uint256 i = 0; i < assets.length; i++) {
            ERC721(assets[i].token).safeTransferFrom(address(this), msg.sender, assets[i].tokenId);
        }
    }

    /**
        @notice Transfers an array of erc721 floor tokens to the msg.sender.
        @param floorTokens The contract addresses for each floor token.
        @param floorTokenIds The token id of each floor token.
     */
    function _transferFloorsOut(address[] memory floorTokens, uint256[] memory floorTokenIds) internal {
        for (uint256 i = 0; i < floorTokens.length; i++) {
            ERC721(floorTokens[i]).safeTransferFrom(address(this), msg.sender, floorTokenIds[i]);
        }
    }

Proof of Concept

Example of attack:

• attacker creates order with a batch of NFTs: one Bored Ape and one MaliciousNFT(only attacker could allow transfer of this NFT)
• Alice takes the attacker's order since it has liquid NFT (Bored Ape) and good conditions (premium size, strike price, etc)
• attacker exercise order, receiving Alice strike
• Alice trying to withdraw her assets, but transaction reverts since transferring of MaliciousNFT was blocked by attacker
• attacker contact Alice and demand a ransom for allowing withdrawal of blocked assets.

Recommended Mitigation Steps

Add functionality that allows withdrawing specific assets apart from a batch.

[outdoteth]

Duplicate: Setting malicious or invalid erc721Assets, erc20Assets or floorTokens prevents the option from being exercised: https://github.com/code-423n4/2022-06-putty-findings/issues/50