DittoETH

Ditto
DeFiFoundryOracle
55,000 USDC
View results
Submission Details
Severity: high
Invalid

DOS attack due to combining short records

Summary

since collateral and ercDebt variables are uint88 in ShortRecord struct when combining the short records together these values get larger and get closer to maximum of uint88, Attacker can apply DOS attack by combining different short records together and prevent bids get matched or created.

Vulnerability Details

Users can call combineShorts function to combine different short records in one short record (all of the valus get added to the target short record collateral,ercDebt...), since collateral and ercDebt variables are uint88 by combining short records these values get closer to maximum of uint88,
if a new bid comes in and get matched with short order the calculated value gets added to the short record's variables and if the target short record is combined short record and its collateral value is very close to maximum of uint88 the added value to collateral will overflow and revert the transaction.

Impact

Imagine attacker creates different big short orders with low prices to get matched with bids and different short records get created quickly,then he combine all of the short records with a short which its order is not fulfilled yet (attacker tries to make collateral value bigger till it gets close to maximum of uint88),

now lets say his remainig order has lowest price in order book and new bid comes in and it tries to match with the order, but since collateral value in short record is close to overflow, the calculated value will overflow when adding it with collateral and the transaction will be reverted.

This prevent new bids get matched with sell orders and also prevent bids from creation since it always revert due to overflow

collateral value is calculated this way each time :

incomingSell.price.mulU88(fillErc).mulU88(LibOrders.convertCR(incomingSell.initialMargin)

(it is a big value and comibing it with different collateral makes it possible to reach the maximum of uint88)

This may also happens if innocent user has combined his/her short records together

Tools Used

Manual Review

Recommendations

in bidMatchAlgo consider a functionalty to check to see if new caluculated value will overflow if it get's added to collateral, you can type cast both new value and collateral to uint256 and add them together and check if its bigger than type(uint88).max and if it is skip the loop with continue

if(uint256(newCollateral) + uint256(collateral) > type(uint88).max){
_shortDirectionHandler(asset, lowestSell, incomingBid, b); // also call this function for setting the b.shortId to the next order
continue;
}

you can also cancel this short record and refund shorter since it can't be matched with any bid order

Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.