DittoETH

Summary

In this protocol we can create short order, for that we need to have in our ethEscrowed balance at least 5 time (initialMargin) the amount of erc*price we want to sell.
Problem is that when matching (either directly with a bid matching our short or lately) the amount of collateral accounted to execute this short is bigger than what is checked when we create the short

Vulnerability Details

Process for creating a short Order in this protocol is like this :

• Call createLimitShort() in ShortOrdersFacet.sol

• Check if a bid can match directly

  • if no bid that can match => LibOrders.sol#addShort() is called , short is added to the OB and eth used to short this erc20 is decremented from shorterAddress : s.vaultUser[vault][order.addr].ethEscrowed -= order.ercAmount.mulU88(order.price).mulU88(initialMargin); and wait for a bid to be matched

  • if a bid can match this order => LibOrders.sol#matchIncomingSell() is called and eth used to short this erc20 is decremented from shoterAddress : s.vaultUser[vault][incomingShort.addr].ethEscrowed -= short.price * short.erc * initialMargin;

• libShortRecord.sol#createShortRecord() is called to created the short record in storage

Let's say a user is sure about the direction of an erc20 token and he wants to short it, for that in this protocol we have to escrow eth at least 5 times the amount of erc20 we want to sell.

but 5 times is ok as Asset.initialMargin == initialCR pass.

Then protocol ensure that this shorter has in fact these eth escrowed in the protocol :

so here protocol is checking that ethEscrowed by user is at least short.price * short.ercAmount * CR

s.vaultUser[Asset.vault][msg.sender].ethEscrowed == short.price * short.erc * 5 is OK (for our example let's say CR = 5)

Problem is when this order is matched, either in LibOrders.sol#matchIncomingShort() or in BidOrdersFacet.sol#matchLowestSell(), collateral accounted when creating the short record in storage is more than what is checked during the creation of it :

• LibOrders.sol#matchIncomingShort() :

so in this case collateral = matchTotal.fillEth = highestBid.price.mulU88(fillErc) + incomingSell.price.mulU88(fillErc).mulU88(incomingSell.initialMargin);

If we rephrase it collateral = bid.price * short.erc + short.price * short.erc * initialMargin
=> so collateral is a lot bigger than what is checked when creating a short (short.price * short.erc * initialMargin)

• In BidOrdersFacet.sol#matchLowestSell() it's approximately the same :

In this case collateral = short.erc * short.price ( 1 + initialMargin)
Same it is a lot larger than collateral amount that user had to own when creating a short and a lot larger than what is decremented the shorter balance, but amount stored in LibShortRecord.createShortRecord() is the amount a lot larger

Then if a user wants to check short collateral he/she has to call LibShortRecord.sol#getCollateralRatio() :

He will have a better collateral than what he is really accounted for.

Moreover the process to exit a short is like this :

1. call ExitShortFacet.sol#exitShortErcEscrowed() or ExitShortFacet.sol#exitShortWallet()

2. Refund ercDebt using wallet or ercEscrowed

3. Get back short.collateral

Example using exitShortErcEscrowed() :

But as we demonstrated above when we create a short and this short is matched our eth balance escrowed is decremented by
ethEscrowed -= short.price * short.ercAmount * CR
but
short.collateral = short.price * short.ercAmount * (1 + CR) or short.collateral = bid.price * short.erc + short.price * short.erc * initialMargin

So if user create a short, is matched and exit it immediately after, ercDebt will be equal to ercAmount and he will gain the difference between short.price * short.ercAmount * CR and short.price * short.ercAmount * (1 + CR) or bid.price * short.erc + short.price * short.erc * initialMargin that will be paid by the protocol.

Impact

As collateral checked to create an order is upper than what is really accounted as collateral to create a short, there is a gap between what a user can own as ethEscrowed and what he/she can account for collateral.
This gap can cause 2 problems :

1. If short user create a short, is matched and exit it immediately

2. Short.collateral real value will have to be filled by TAPP(protocol) in case of full liquidation

Both these impacts represent protocol loss's

Tools Used

VSCode

Recommendations

Correct the amount decremented from the shorter as the real amount accounted for collateral.

Correct the amount checked during short creating