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

Short record will stay flagged even after modifying in certain scenario

T1MOH

Summary

When shortRecord's collateralRatio is lower than primaryThreshold, liquidators can flag shortRecord. It means that user has time to increase collRatio. As collRatio reaches primaryThreshold in this time window, flag must reset.

However flag doesn't reset in case when user's short order is executed while user already has max number of shortRecords (256). When user has max number of shortRecords, last shortRecord is modified instead of creating new record.

Assume last shortRecord's ratio is low, it's flagged. User executes short order and that last shortRecord is modified. Now collRatio is higher than primaryThreshold and flag must reset, but that doesn't happen. There is no direct external method to resetFlag, hence user's last shortRecord will be liquidated as soon as collRatio becomes lower than primaryThreshold. And now it will happen immediately without grace period.

Vulnerability Details

When bid matches short order, new shortRecord is created.

function matchlowestSell(
address asset,
STypes.Order memory lowestSell,
STypes.Order memory incomingBid,
MTypes.Match memory matchTotal
) private {
uint88 fillErc = incomingBid.ercAmount > lowestSell.ercAmount
? lowestSell.ercAmount
: incomingBid.ercAmount;
uint88 fillEth = lowestSell.price.mulU88(fillErc);
​
if (lowestSell.orderType == O.LimitShort) {
​
...
​
if (lowestSell.shortRecordId > 0) {
...
} else {
// shortRecord newly created
lowestSell.shortRecordId = LibShortRecord.createShortRecord(
asset,
lowestSell.addr,
status,
shortFillEth,
fillErc,
matchTotal.ercDebtRate,
matchTotal.zethYieldRate,
0
);
}
}
​
...
}

Take a look on how shortRecord is created. It modifies shortRecord with max id if there are to many shortRecords:

function createShortRecord(
address asset,
address shorter,
SR status,
uint88 collateral,
uint88 ercAmount,
uint64 ercDebtRate,
uint80 zethYieldRate,
uint40 tokenId
) internal returns (uint8 id) {
...
​
if (id <= Constants.SHORT_MAX_ID) {
...
} else {
@> // All shortRecordIds used, combine into max shortRecordId
id = Constants.SHORT_MAX_ID;
@> fillShortRecord(
asset,
shorter,
id,
status,
collateral,
ercAmount,
ercDebtRate,
zethYieldRate
);
}
}

However that modified short record could be flagged. As you can see new collRatio is not checked and not resetted. It means that if collRatio will decrease in future, record will be liquidated without grace period

Impact

Last shortRecord's flag doesn't reset on fulfilling when collRatio reaches primaryThreshold. User is subject to immediate liquidation in future without grace period.

Tools Used

Manual Review

Recommendations

Safe scenario is to call maybeResetFlag() on every filling of shortRecord:

function fillShortRecord(
address asset,
address shorter,
uint8 shortId,
SR status,
uint88 collateral,
uint88 ercAmount,
uint256 ercDebtRate,
uint256 zethYieldRate
) internal {
AppStorage storage s = appStorage();
​
uint256 ercDebtSocialized = ercAmount.mul(ercDebtRate);
uint256 yield = collateral.mul(zethYieldRate);
​
STypes.ShortRecord storage short = s.shortRecords[asset][shorter][shortId];
if (short.status == SR.Cancelled) {
short.ercDebt = short.collateral = 0;
}
​
short.status = status;
LibShortRecord.merge(
short,
ercAmount,
ercDebtSocialized,
collateral,
yield,
LibOrders.getOffsetTimeHours()
);
+ LibShortRecord.maybeResetFlag(short, asset);
}

But you can just call it in createShortRecord():

function createShortRecord(
address asset,
address shorter,
SR status,
uint88 collateral,
uint88 ercAmount,
uint64 ercDebtRate,
uint80 zethYieldRate,
uint40 tokenId
) internal returns (uint8 id) {
...
​
else {
// All shortRecordIds used, combine into max shortRecordId
id = Constants.SHORT_MAX_ID;
fillShortRecord(
asset,
shorter,
id,
status,
collateral,
ercAmount,
ercDebtRate,
zethYieldRate
);
+ LibShortRecord.maybeResetFlag(short, asset);
}
}
Updates

Lead Judging Commences

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

finding-176

Support

FAQs

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