GalloDaSballo
commented
2 years ago An order being set doesn't have the guarantee of being cancellable, that is true.
But that's a lot closer to expecting the ability to cancel a tx mid flight, which we don't normally consider a real security, we all understand that signing the tx means the tx is out
I think I'll downgrade and believe Low was appropriate for Putty as well.
Also there's a way to cancel orders without relying on DB, specifically the creator has privileged access to the hash (as they can compute it before hand), and they also can incrementNonce as an alternative to blanket cancel.
Severity looks inflated
blur-io-toad
Lines of code
https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/BlurExchange.sol#L181 https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/BlurExchange.sol#L198
Vulnerability details
Impact
Order cancellation requires makers to call cancel(), inputting the order as a function parameter. or call cancelOrders and input a list of order,
the method can cause cause two issues.
This first issue is that it is an on-chain signal for MEV users to frontrun the cancellation and fill the order.
When user can call incrmentNounce for sure, however, such transaction is still vulnerable to frontr-running bot, the MEV bot can detect incrementNounce and fill the order before the nonce is incremented.
The second issue is the dependency to a centralized service for cancelling the order. As orders are signed off chain, they would be stored in a centralized database. It is unlikely that an end user would locally record all the orders they make. This means that when cancelling an order, maker needs to request the order parameters from the centralized service. If the centralized service goes offline, it could allow malicious parties who have a copy of the order database to fill orders that would have been cancelled otherwise.
Proof of Concept
Bob signs an order which gets recorded in Blur marketplace servers.
Alice mirrors all the orders using Putty APIs.
Blur marketplace servers. servers go offline.
Bob wants to cancel his order because changing token prices makes his order less favourable to him.
Bob cannot cancel his order because Putty servers are down and he does not remember the exact amounts of tokens he used.
Alice goes through all the orders in her local mirror and fulfills the non-cancelled orders, including Bob’s, with extremely favourable terms for herself.
Tools Used
Recommended Mitigation Steps
https://github.com/ProjectOpenSea/seaport/blob/171f2cd7faf13b2bf0455851499f1981274977f7/contracts/lib/CounterManager.sol
to achieve bulk cancelling is using counters. For example, Seaport uses counters, which is an extra order parameter that has to match the corresponding counter state variable. It allows maker to cancel all his orders by incrementing the counter state variable by one.
https://github.com/ProjectOpenSea/seaport/blob/171f2cd7faf13b2bf0455851499f1981274977f7/contracts/lib/Consideration.sol#L475-L478
Source: https://code4rena.com/reports/2022-06-putty#m-14-order-cancellation-is-prone-to-frontrunning-and-is-dependent-on-a-centralized-database