c4-bot-3 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOrders.sol#L600-L615

Vulnerability details

Proof of Concept

Whenever an asked bid is to be matched the sellMatchAlgo() gets called, now take a look at this section of the function https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOrders.sol#L600-L615

                    if (incomingAsk.ercAmount == highestBid.ercAmount) {
                        matchOrder(s.bids, asset, highestBid.id);
                        updateBidOrdersOnMatch(s.bids, asset, highestBid.id, true);
                    } else {
                        highestBid.ercAmount -= incomingAsk.ercAmount;
                        s.bids[asset][highestBid.id].ercAmount = highestBid.ercAmount;
                        updateBidOrdersOnMatch(s.bids, asset, highestBid.id, false);
                        // Check reduced dust threshold for existing limit orders
                        if (highestBid.ercAmount.mul(highestBid.price) < LibAsset.minBidEth(asset).mul(C.DUST_FACTOR)) {
                            cancelBid(asset, highestBid.id);
                        }
                    }
                    incomingAsk.ercAmount = 0;
                    matchIncomingSell(asset, incomingAsk, matchTotal);
                    return;
                }

This essectially checks to ensure that after matching the ask not less than dust is added back to the order book, i.e the else block since the if block is for when the amounts match, however the calculation to check for the remaining amount not being less than the minimum accepted value is flawed, because of an additional multiplication to C_DUST_FACTOR which is 0.5 ether.

The right check should be if (highestBid.ercAmount.mul(highestBid.price) < LibAsset.minBidEth(asset), but with the addition of the multiplication to C_DUST_FACTOR which in short is 0.5 ether this heavily overvalues the right side of the equation and makes bids that've beeen matched that didn't leave less than the minimum accepted eth value back in the order book to have the transaction revert

Impact

Valid attempts would fail causing DOS at matching asks since now the minEther is very high

Recommended Mitigation Steps

Consider not multiplying to C_DUST_FACTOR in sellMatchAlgo() and instead just check if (highestBid.ercAmount.mul(highestBid.price) < LibAsset.minBidEth(asset)

Assessed type

Error

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 primary issue

raymondfam commented 3 months ago

Devoid of numerical examples to support your claim.

c4-judge commented 3 months ago

hansfriese marked the issue as unsatisfactory: Insufficient proof

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