OrderBook

Finding Title

Missing Token Allowance Check in amendSellOrder() Allows Previously Created Orders for Delisted Tokens to Persist

Summary

The amendSellOrder() function allows a seller to modify an existing order. However, it fails to verify if the token being sold (order.tokenToSell) is still in the allowedSellToken mapping. This oversight allows a seller to amend and effectively keep their order active in the marketplace, even after the protocol owner has explicitly delisted the token via setAllowedSellToken(..., false). This undermines the owner's administrative control and contradicts the intended function of the token whitelist.

Finding Description

The protocol owner has the ability to manage which tokens are permitted for trading using the setAllowedSellToken(address _token, bool _isAllowed) function. When a new order is created, the createSellOrder() function correctly checks if the token is allowed:

However, this crucial check is missing in the amendSellOrder() function. If an order was created when a token was allowed, and the owner later decides to delist that token for security or other reasons, the existing order remains active. A seller can then call amendSellOrder() to change its price or deadline, effectively refreshing it and keeping a potentially unwanted or unsafe token listed for sale.

Scenario of Exploit:

1. Token A is initially whitelisted by the owner.

2. A seller creates an order to sell 100 units of Token A.

3. The owner discovers a vulnerability in Token A and decides to delist it by calling setAllowedSellToken(address(TokenA), false).

4. No new orders for Token A can be created, which is the intended behavior.

5. However, the original seller can still call amendSellOrder() on their existing order. They can update the price and extend the deadline, keeping the vulnerable token available for purchase by unsuspecting buyers. The protocol fails to enforce the delisting on existing, amendable orders.

Impact

This vulnerability directly undermines the protocol's primary safety mechanism for managing supported assets. The consequences include:

• Exposure to Risky Assets: Buyers may be exposed to purchasing tokens that the protocol owner has deemed unsafe, insecure, or no longer viable.

• Incomplete Administrative Control: The owner's ability to manage the market is incomplete. Delisting a token does not fully remove it from active trading as intended, creating a discrepancy between the owner's actions and the protocol's state.

• Erosion of Trust: Users may lose trust in the protocol's curation and safety measures if they find that delisted tokens are still actively being traded.

Likelihood

Medium. The scenario requires an owner to delist a token while active orders for it still exist, which is a plausible administrative action. A seller of such an order can then easily perform this action without any special knowledge.

Proof of Concept

The following test demonstrates that an order for a delisted token can be successfully amended.

Test File: test/PoC/AmendDelistedToken.t.sol

Execution Command & Result:
The test was executed using the command forge test --match-path test/PoC/AmendDelistedToken.t.sol. The test passed, confirming the vulnerability.

The successful result ([PASS]) proves that the call to amendSellOrder for the delisted token did not revert and successfully updated the order's state, which is incorrect behavior.

Recommended Mitigation

Add a validation check at the beginning of the amendSellOrder function to ensure the token associated with the order is still allowed for trading.

This simple check ensures that the owner's administrative action of delisting a token is enforced consistently across all functions, preventing unwanted assets from persisting in the marketplace.