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

0 stars 0 forks source link

Lack of Finalization for Partially Filled Asks #279

Closed c4-bot-10 closed 3 months ago

c4-bot-10 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/main/contracts/facets/BidOrdersFacet.sol#L224

Vulnerability details

Impact

The matchlowestSell function does not account for updating the ercAmount of ask orders that are only partially filled by incoming bids, potentially leading to inaccuracies in the displayed order book.

Proof of Concept

Github:

if (lowestSell.orderType == O.LimitShort) {
// ...
} else {
    // Match ask
    s.vaultUser[s.asset[asset].vault][lowestSell.addr].ethEscrowed += fillEth;
    matchTotal.askFillErc += fillErc;
}
matchTotal.fillErc += fillErc;
matchTotal.fillEth += fillEth;
matchTotal.lastMatchPrice = lowestSell.price;

Scenario Steps:

  1. An incoming bid order is placed that matches with an existing ask order, but only partially fills it.

  2. The matchlowestSell function is executed for the incoming bid, matching it against the lowest available ask order.

  3. In the scenario where an ask order is partially filled, the matchlowestSell function correctly updates ethEscrowed for the seller and records the filled ERC amount in matchTotal.askFillErc. However, it fails to adjust the remaining ercAmount for the ask order that remains on the market. This oversight means the order book could display incorrect information about the available quantity for that ask order, potentially misleading future matching attempts.

  4. The ask order remains in the order book with its original ercAmount, not reflecting the partial fill. This could lead to mismatches in the order book, affecting market integrity and participant decision-making.

Tools Used

Manual

Recommended Mitigation Steps

Implement a mechanism within matchlowestSell or at an appropriate point following a partial match, to update the remaining ercAmount for partially filled ask orders.

Assessed type

Context

c4-pre-sort commented 3 months ago

raymondfam marked the issue as insufficient quality report

raymondfam commented 3 months ago

Unstructured/insufficient proof.

c4-pre-sort commented 3 months ago

raymondfam marked the issue as primary issue

c4-judge commented 3 months ago

hansfriese marked the issue as unsatisfactory: Insufficient proof