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

Margin call doesn't distribute yield to TAPP

nican0r

Summary

During a margin call the yield is supposed to be distributed to the TAPP in the call to LibShortRecord::disburseCollateral according to the natspec, but this only happens if the short has been updated within Constants.YIELD_DELAY_HOURS = 1 hour , given that the minimum liquidation period is 10 hours from the last update made by the user and there’s nothing else in the MarginCallPrimaryFacet::liquidate function that changes the updatedAt time, instead of distributing yield to the TAPP it gets sent to the short being liquidated.

Vulnerability Details

A margin call made using MarginCallPrimaryFacet::liquidate makes a call to the _fullorPartialLiquidation function which calls LibShortRecord::disburseCollateral to distribute the yield from the short position. The natspec above disburseCollateral indicates that a user should lose their yield to the TAPP:

/*
@dev If somebody exits a short, gets margin called, decreases their collateral before YIELD_DELAY_HOURS duration is up,
they lose their yield to the TAPP
*/
bool isNotRecentlyModified = LibOrders.getOffsetTimeHours() - updatedAt > Constants.YIELD_DELAY_HOURS;
if (isNotRecentlyModified) {
s.vaultUser[vault][shorter].ethEscrowed += yield;
} else {
s.vaultUser[vault][address(this)].ethEscrowed += yield;
}

but because short.updatedAt must be at least 10 for a user to call liquidate, which is > YIELD_DELAY_HOURS (1) and the time to allow liquidation is counted from the last update to the short this guarantees that for a liquidator to call liquidate at least 10 hours will have to have passed from the last update:

MarginCallPrimaryFacet::_canLiquidate

uint256 timeDiff = LibOrders.getOffsetTimeHours() - m.short.updatedAt;
uint256 resetLiquidationTime = LibAsset.resetLiquidationTime(m.asset);
​
​
if (timeDiff >= resetLiquidationTime) {
return false;
} else {
uint256 secondLiquidationTime = LibAsset.secondLiquidationTime(m.asset);
bool isBetweenFirstAndSecondLiquidationTime = timeDiff
> LibAsset.firstLiquidationTime(m.asset) && timeDiff <= secondLiquidationTime
&& s.flagMapping[m.short.flaggerId] == msg.sender;
...
​
​
return true;

the isNotRecentlyModified boolean will be true and therefore add the yield to the ethEscrowed of the user instead of the TAPP.

Impact

Shorters always receive the yield from their position when being liquidated.

Tools Used

Manual Review

Recommendations

Add a boolean liquidating to ensure that the yield gets sent to the TAPP during liquidations:

function disburseCollateral(
address asset,
address shorter,
uint88 collateral,
uint256 zethYieldRate,
uint24 updatedAt,
+ bool liquidating
) internal {
...
bool isNotRecentlyModified = LibOrders.getOffsetTimeHours() - updatedAt > Constants.YIELD_DELAY_HOURS;
- if (isNotRecentlyModified) {
+ if (isNotRecentlyModified && !liquidating) {
s.vaultUser[vault][shorter].ethEscrowed += yield;
} else {
s.vaultUser[vault][address(this)].ethEscrowed += yield;
}
}
}

MarginCallPrimaryFacet::_fullorPartialLiquidation L221, L236:

LibShortRecord.disburseCollateral(
m.asset,
m.shorter,
m.short.collateral,
m.short.zethYieldRate,
m.short.updatedAt,
+true
);
Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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