search

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

0 stars 0 forks source link

cancelShort will cause users loss of funds #240

Closed c4-bot-5 closed 4 months ago

c4-bot-5 commented 5 months ago

Lines of code

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

Vulnerability details

Impact

cancelShort will not refund a shorter after his short being canceled.

Proof of Concept

When a user add a short order using createLimitShort method, LibOrders.addShort will be called in case incomingShort.price < p.oraclePrice.

LibOrders.addShort(asset, incomingShort, orderHintArray);

In addShort, eth value will be deducted from the shorter's ethEscrowed at s.vaultUser[vault][order.addr].ethEscrowed -= eth

The issue here that cancelShort will not refund ethEscrowed that's taken from the shorter if the short is closed.

What exactly cancelShort do:
cancelShort will call LibOrders.cancelShort, if (shortRecord.status == SR.Closed) deleteShortRecord will be called for the shortRecordId, Inside deleteShortRecord method there is no refund for ethEscrowed that was taken from the shorter balance when the short was added in [s.vaultUser[vault][order.addr].ethEscrowed -= eth].

Remember that when a short order is added, the status of the shortRecord initiated as SR.Closed.

Tools Used

Manual review

Recommended Mitigation Steps

Refund the shorter the amount that was deducted from ethEscrowed on cancelShort

Assessed type

Other

c4-pre-sort commented 5 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 5 months ago

raymondfam marked the issue as primary issue

raymondfam commented 5 months ago

Inadequate elaboration and proof given.

hansfriese commented 4 months ago

It adds here.

c4-judge commented 4 months ago

hansfriese marked the issue as unsatisfactory: Invalid