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

Using a cached price in the critical shutdownMarket()

serialcoder

Summary

The MarketShutdownFacet::shutdownMarket() is a critical function allowing anyone to freeze the market permanently. The function determines whether or not the market will be frozen based on the asset collateral ratio calculated from a cached price, which can be outdated (too risky for this critical function).

Once the market is frozen, no one can unfreeze it.

Vulnerability Details

The shutdownMarket() allows anyone to call to freeze the market permanently when the asset collateral ratio threshold (default of 1.1 ether) has been reached. Once the market is frozen, all shorters will lose access to their positions. Even the protocol's DAO or admin cannot unfreeze the market. Therefore, the shutdownMarket() becomes one of the most critical functions.

To calculate the asset collateral ratio (cRatio), the shutdownMarket() executes the _getAssetCollateralRatio(). However, the _getAssetCollateralRatio() calculates the cRatio using the cached price loaded from the LibOracle::getPrice().

Using the cached price in a critical function like shutdownMarket() is too risky, as the cached price can be outdated. The function should consider only a fresh price queried from Chainlink.

function shutdownMarket(address asset)
external
onlyValidAsset(asset)
isNotFrozen(asset)
nonReentrant
{
@> uint256 cRatio = _getAssetCollateralRatio(asset);
@> if (cRatio > LibAsset.minimumCR(asset)) {
revert Errors.SufficientCollateral();
} else {
STypes.Asset storage Asset = s.asset[asset];
uint256 vault = Asset.vault;
uint88 assetZethCollateral = Asset.zethCollateral;
s.vault[vault].zethCollateral -= assetZethCollateral;
@> Asset.frozen = F.Permanent;
if (cRatio > 1 ether) {
// More than enough collateral to redeem ERC 1:1, send extras to TAPP
uint88 excessZeth =
assetZethCollateral - assetZethCollateral.divU88(cRatio);
s.vaultUser[vault][address(this)].ethEscrowed += excessZeth;
// Reduces c-ratio to 1
Asset.zethCollateral -= excessZeth;
}
}
emit Events.ShutdownMarket(asset);
}
​
...
​
function _getAssetCollateralRatio(address asset)
private
view
returns (uint256 cRatio)
{
STypes.Asset storage Asset = s.asset[asset];
@> return Asset.zethCollateral.div(LibOracle.getPrice(asset).mul(Asset.ercDebt));
}

  • https://github.com/Cyfrin/2023-09-ditto/blob/a93b4276420a092913f43169a353a6198d3c21b9/contracts/facets/MarketShutdownFacet.sol#L36

  • https://github.com/Cyfrin/2023-09-ditto/blob/a93b4276420a092913f43169a353a6198d3c21b9/contracts/facets/MarketShutdownFacet.sol#L37

  • https://github.com/Cyfrin/2023-09-ditto/blob/a93b4276420a092913f43169a353a6198d3c21b9/contracts/facets/MarketShutdownFacet.sol#L44

  • https://github.com/Cyfrin/2023-09-ditto/blob/a93b4276420a092913f43169a353a6198d3c21b9/contracts/facets/MarketShutdownFacet.sol#L99

Impact

Using the cached price in a critical function like shutdownMarket() is too risky, as the cached price can be outdated.

Once the market is frozen, all shorters will lose access to their positions. Even the protocol's DAO or admin cannot unfreeze the market.

Tools Used

Manual Review

Recommendations

The shutdownMarket() requires the most accurate price, not just a cached price. Execute the LibOracle::getOraclePrice() to get the accurate price from Chainlink.

Updates

Lead Judging Commences

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

Support

FAQs

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