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

Decreasing and increasing a short's collateral potentially uses an outdated asset price to calculate the collateral ratio

bernd

Summary

The decreaseCollateral and increaseCollateral functions in the ShortRecordFacet contract calculate the short's collateral ratio based on the cached asset price, which may be outdated, leading to a divergence between the actual collateral ratio (based on the asset spot price) and the calculated collateral ratio.

Vulnerability Details

According to the conditions for updating the oracle, decreasing the short's collateral via the ShortRecordFacet.decreaseCollateral function should update the oracle price if the oracle price is older than 15 minutes.

However, in the current implementation of the decreaseCollateral function, the short's collateral ratio, cRatio, is calculated by calling the getCollateralRatio function in line 94:

082: function decreaseCollateral(address asset, uint8 id, uint88 amount)
083: external
084: isNotFrozen(asset)
085: nonReentrant
086: onlyValidShortRecord(asset, msg.sender, id)
087: {
088: STypes.ShortRecord storage short = s.shortRecords[asset][msg.sender][id];
089: short.updateErcDebt(asset);
090: if (amount > short.collateral) revert Errors.InsufficientCollateral();
091:
092: short.collateral -= amount;
093:
094: ❌ uint256 cRatio = short.getCollateralRatio(asset);
095: if (cRatio < LibAsset.initialMargin(asset)) {
096: revert Errors.CollateralLowerThanMin();
097: }
098:
099: uint256 vault = s.asset[asset].vault;
100: s.vaultUser[vault][msg.sender].ethEscrowed += amount;
101:
102: LibShortRecord.disburseCollateral(
103: asset, msg.sender, amount, short.zethYieldRate, short.updatedAt
104: );
105: emit Events.DecreaseCollateral(asset, msg.sender, id, amount);
106: }

The called getCollateralRatio function uses the LibOracle.getPrice function to calculate the collateral ratio:

22: function getCollateralRatio(STypes.ShortRecord memory short, address asset)
23: internal
24: view
25: returns (uint256 cRatio)
26: {
27: return short.collateral.div(short.ercDebt.mul(LibOracle.getPrice(asset)));
28: }

The LibOracle.getPrice function returns the currently cached asset price, which potentially is older than 15 minutes.

147: function getPrice(address asset) internal view returns (uint80 oraclePrice) {
148: AppStorage storage s = appStorage();
149: return uint80(s.bids[asset][Constants.HEAD].ercAmount);
150: }

Consequently, the calculated cRatio in line 94 of the decreaseCollateral function is based on the potentially outdated asset price, resulting in the collateral ratio being inaccurate and diverging from the actual collateral ratio based on the current asset spot price.

A short owner can exploit this by decreasing the short's collateral up to the point where the resulting collateral ratio is equal to the initial margin (i.e., 500%). As the collateral ratio, cRatio, is calculated in line 94 based on the outdated cached oracle price, the short owner can withdraw more collateral than the actual collateral ratio (based on the asset spot price) would allow.

Similarly, the increaseCollateral function is affected as well.

Impact

Short-position owners can withdraw more collateral than eligible, negatively affecting the overall asset's collateral ratio.

Tools Used

Manual Review

Recommendations

Consider using the LibOracle.getSavedOrSpotOraclePrice function together with the getCollateralRatioSpotPrice function to calculate the collateral ratio based on the current asset price.

Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other
bernd Submitter
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-567

Support

FAQs

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