DittoETH
Ditto
DeFiFoundryOracle
55,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

First Shorts in Orderbook may be skipped when matching.

836541

Summary

The function _updateOracleAndStartingShort updates the startingShortId, so matching algorithm knows where to start the matching iteration in Shorts Orderbook. A parameter of this method is shortHintArray, which is a array of hints of where the startingShortId should be. The docs say that a hint can have a price 1% higher than the actual startingShortId and when this happens the logic should go back until it finds the real startingShortId. However, the function doesn't search the real startingShortId, but simply sets startingShortId to be the first value which is in 1% price range.

Vulnerability Details

  1. In LibOrders.sol, this is _updateOracleAndStartingShort:

function _updateOracleAndStartingShort(address asset, uint16[] memory shortHintArray)
private
{
AppStorage storage s = appStorage();
uint256 oraclePrice = LibOracle.getOraclePrice(asset);
uint256 savedPrice = asset.getPrice();
asset.setPriceAndTime(oraclePrice, getOffsetTime());
bool shortOrdersIsEmpty = s.shorts[asset][Constants.HEAD].nextId == Constants.TAIL;
if (shortOrdersIsEmpty) {
s.asset[asset].startingShortId = Constants.HEAD;
} else {
if (oraclePrice == savedPrice) {
return;
}
uint16 shortHintId;
for (uint256 i = 0; i < shortHintArray.length;) {
shortHintId = shortHintArray[i];
unchecked {
++i;
}
​
{
O shortOrderType = s.shorts[asset][shortHintId].orderType;
if (
shortOrderType == O.Cancelled || shortOrderType == O.Matched
|| shortOrderType == O.Uninitialized
) {
continue;
}
}
​
uint80 shortPrice = s.shorts[asset][shortHintId].price;
uint16 prevId = s.shorts[asset][shortHintId].prevId;
//@dev: force hint to be within 1% of oracleprice
bool startingShortWithinOracleRange = shortPrice
<= oraclePrice.mul(1.01 ether)
&& s.shorts[asset][prevId].price >= oraclePrice;
bool isExactStartingShort = shortPrice >= oraclePrice
&& s.shorts[asset][prevId].price < oraclePrice;
bool allShortUnderOraclePrice = shortPrice < oraclePrice
&& s.shorts[asset][shortHintId].nextId == Constants.TAIL;
​
if (startingShortWithinOracleRange || isExactStartingShort) {
//@dev only consider the x% above oraclePrice if there are prev Shorts with price >= oraclePrice
s.asset[asset].startingShortId = shortHintId;
return;
} else if (allShortUnderOraclePrice) {
s.asset[asset].startingShortId = Constants.HEAD;
return;
}
}
​
revert Errors.BadShortHint();
}
}

  1. The important snippet is:

if (startingShortWithinOracleRange || isExactStartingShort) {
//@dev only consider the x% above oraclePrice if there are prev Shorts with price >= oraclePrice
s.asset[asset].startingShortId = shortHintId;
return;

As you can see, if the shortHint is true in startingShortWithinOracleRange, which is the 1% price range, the logic sets the startingShortId of this stable coin as shortHint and return, exiting the function. This is not what was intended.

Impact

Severity: Medium.
Probability: Medium.
Impact: Medium.

Finding the exact startingShortId can be hard for a user. So, users will often input a shortHint in 1% price range and the actual best prices can take longer than expected to match. This is has three problems:

  1. When shorts and asks have the same price, ask is chosen. However, as startingShortId may target a short with a higher price than it should, sometimes even if ask has the best market price, short can be matched instead of the ask.

  2. Shorts with best price may take longer to get matched, which is unfair and can be seen as a denial of service for users with the best shorts.

  3. Not what was expected in the docs: "If startingShortId is valid within that range but is not exact, the orderbook will match downwards until it hits the true startingShortId. Once it hits that, the system will match back upwards and will behave like a normal orderbook again. This is to allow the next order to set the new oracle price within some reasonable range of values when the oracle price needs to change (freshness)."

Tools Used

Manual Review

Recommendations

Implement the logic that was expected by the docs "the orderbook will match downwards until it hits the true ".

Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-494

T1MOH Auditor
almost 2 years ago
0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other
Assigned finding tags:

finding-494

Support

FAQs

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