Bahurum - Reentrancy in `settleContract` with ERC777 `asset`

[sherlock-admin]

Bahurum

high

Reentrancy in settleContract with ERC777 asset

Summary

In settleContract(), settledContracts mapping is updated after transfer of tokens to the bear, which allows reentrancy with ERC777 asset tokens. As a result, the bear can transfer multiple NFTs to the bull and get from the bull an amount of tokens up to the bull's allowance to the contract.

Vulnerability Detail

1. bear creates an order
2. bull matches the order
3. bear calls settleContract with a floor tokenId. NFT with tokenId is transfered to bull, bear receives payment of ERC777:
   • On ERC777 callback the bear calls again settleContract with another floor tokenId. Note that settledContracts[contractId] is still false and the check at line 389 passes. The second NFT is transfered to the bull and if the bull has enough allowance to the contract, bearAssetAmount is transfered again to the bear. In the transfer the bear can reenter again and repeat until allowance of the bull to the contract is too low.

Impact

A bear can settle the same contract with many floor NFTs in its possession, and get in exchange almost all of the bull's tokens allowed to the contract.

Code Snippet

    function settleContract(Order calldata order, uint tokenId) public nonReentrant {
        bytes32 orderHash = hashOrder(order);

        // ContractId
        uint contractId = uint(orderHash);

        address bear = bears[contractId];

        // Check that only the bear can settle the contract
        require(msg.sender == bear, "ONLY_BEAR");

        // Check that the contract is not expired
        require(block.timestamp < order.expiry, "EXPIRED_CONTRACT");

        // Check that the contract is not already settled
        require(!settledContracts[contractId], "SETTLED_CONTRACT");

        address bull = bulls[contractId];

        // Try to transfer the NFT to the bull (needed in case of a malicious bull that block transfers)
        try IERC721(order.collection).safeTransferFrom(bear, bull, tokenId) {}
        catch (bytes memory) {
            // Transfer NFT to BvbProtocol
            IERC721(order.collection).safeTransferFrom(bear, address(this), tokenId);
            // Store that the bull has to retrieve it
            withdrawableCollectionTokenId[order.collection][tokenId] = bull;
        }

        uint bearAssetAmount = order.premium + order.collateral;
        if (bearAssetAmount > 0) {
            // Transfer payment tokens to the Bear
            IERC20(order.asset).safeTransfer(bear, bearAssetAmount);
        }

        settledContracts[contractId] = true;

        emit SettledContract(orderHash, tokenId, order);
    }

Tool used

Manual Review

Recommendation

Move settledContracts[contractId] = true; before the token transfer:

        ...
+       settledContracts[contractId] = true;
        uint bearAssetAmount = order.premium + order.collateral;

        if (bearAssetAmount > 0) {
            // Transfer payment tokens to the Bear
            IERC20(order.asset).safeTransfer(bear, bearAssetAmount);
        }

-       settledContracts[contractId] = true;
        ...