code-423n4 / 2024-03-dittoeth-findings

0 stars 0 forks source link

Hint Array Inaccuracy Post-Reorganization #280

Closed c4-bot-5 closed 3 months ago

c4-bot-5 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/BidOrdersFacet.sol#L40

Vulnerability details

Impact

Blockchain reorganizations (reorgs) can lead to a scenario where the transactions that were considered final get replaced by alternative transactions. If these replaced transactions involve order placements, cancellations, or matches, the hint arrays passed to the functions above might no longer accurately represent the order book's state.

In the worst-case scenario, transactions might fail if the hint array suggests an order operation that is no longer valid, such as matching an order that has been canceled or filled by transactions included in the reorg.

Proof of Concept

The issue primarily affects the functions and mechanisms that rely on hint arrays for optimizing gas usage during order placement and matching. This includes the following parts of the code

These functions use hint arrays to efficiently place or match orders by providing a pre-sorted array of order IDs that reduce the need for on-chain sorting.

Tools Used

Manual

Recommended Mitigation Steps

Introduce a mechanism that validates hint arrays against the current state of the order book before proceeding with any order placement or matching. This would involve checking if the hint array still represents a valid path through the order book. If not, the transaction could revert or fall back to a less gas-optimized path that does not rely on the hint array.

Assessed type

Context

c4-pre-sort commented 3 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 3 months ago

raymondfam marked the issue as duplicate of #62

raymondfam commented 3 months ago

Chain reorg not applicable to mainnet where the contracts will be deployed to.

c4-judge commented 3 months ago

hansfriese marked the issue as unsatisfactory: Invalid

anthonyshiks commented 3 months ago

Hi I believe this is valid issue given 43nalyzer report m4 is valid.

anthonyshiks commented 3 months ago

https://github.com/code-423n4/2024-03-dittoeth/blob/main/4naly3er-report.md

hansfriese commented 3 months ago
  1. PJQA was closed already.
  2. M4 should be invalidated. It's just an automatic finding.
  3. The original severity of this issue is QA at best with the non-critical impact(no fund loss) + rare likelihood.
anthonyshiks commented 3 months ago

Okay noted.