carrot - Missing settled orders check

[sherlock-admin]

carrot

high

Missing settled orders check

Summary

There are three states which mark the end of an order: settled, reclaimed and cancelled. When matching an order, the checkIsValidOrder function makes checks to ensure that:

1. reclaimed orders aren't matched. This is ensured by checking order.expiry
2. cancelled orders aren't matched. This is ensured by checking !canceledOrders[] require statement

It is never checked before matching if an offer has already been settled.

Vulnerability Detail

Attacker does the following steps:

1. Create order as a bear.
2. Match against their own order as the bull.
3. Settle the order by calling settleContract. This transfer NFTs to themself, along with collateral. Net cost = fees. This also sets settledContracts[contractId] = true;
4. Transfer away ownership to address(0) from the bull's side. This sets bulls[uint(orderHash)] = address(0)
5. Have another bull match. A bull can match since the bull of this order is address(0), which bypasses all the checks in checkIsValidOrder function
6. This bull has paid the premium, but cannot ever call reclaimContract function since the require(!settledContracts[contractId], "SETTLED_CONTRACT"); will never pass as the contract is already marked as settled.
7. The bear can also never settle the order, but their cost is very low (only the premium). This is essentially a stuck order

   Impact

   Trap orders which can never be settled or reclaimed, losing the bull their premium

   Code Snippet

   https://github.com/sherlock-audit/2022-11-bullvbear/blob/main/bvb-protocol/src/BvbProtocol.sol#L734-L761

   Tool used

Manual Review

Recommendation

Add a require statement to check for settled orders in checkIsValidOrder function require(!settledContracts[uint(orderHash)], "Order already settled");

Duplicate of #114

[datschill]

PR fixing this issue : https://github.com/BullvBear/bvb-solidity/pull/1

[datschill]

PR fixing the transfer to 0x0 : https://github.com/BullvBear/bvb-solidity/pull/3