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

ERC redemption not done at oracle price market shutdown

aballok

Summary

Functionality is not following intended specifications. When a market is permanently shutdown its only fair to ensure erc redemptions are at the prices that prevailed at shutdown this is not the case.

Vulnerability Details

The project documentation in the comments clearly specifies that oracle price at shutdown will be used. See below
MarketShutdownFacet.sol lines 58-62 comments below

/**
* @notice Allows user to redeem erc from their wallet and/or escrow at the oracle price of market shutdown
* @dev Market must be permanently frozen, redemptions drawn from the combined collateral of all short records
*
* @param asset The market that will be impacted
*/
​

However studying the shutdown function below shows that oracle prices at shutdown are not captured or stored.

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);
}

There is no logic in the above function that captures or gets prices from oracles when above function is called so that prices at shutdown can be saved. Actually oracle prices are obtained when redeeming erc see function below

function redeemErc(address asset, uint88 amtWallet, uint88 amtEscrow)
external
isPermanentlyFrozen(asset)
nonReentrant
{
if (amtWallet > 0) {
asset.burnMsgSenderDebt(amtWallet);
}
​
if (amtEscrow > 0) {
s.assetUser[asset][msg.sender].ercEscrowed -= amtEscrow;
}
​
uint88 amtErc = amtWallet + amtEscrow;
uint256 cRatio = _getAssetCollateralRatio(asset);
// Discount redemption when asset is undercollateralized
uint88 amtZeth = amtErc.mulU88(LibOracle.getPrice(asset)).mulU88(cRatio); // <- price here
s.vaultUser[s.asset[asset].vault][msg.sender].ethEscrowed += amtZeth;
emit Events.RedeemErc(asset, msg.sender, amtWallet, amtEscrow);
}

The above LibOracle.getPrice => gets the price that was stored in the protocol for the oracle price for erc.
However to keep this price updated a call to LibOracle.setPriceAndTime will need to have been done.

//LibOracle.sol functions
function setPriceAndTime(address asset, uint256 oraclePrice, uint32 oracleTime)
internal
{
AppStorage storage s = appStorage();
s.bids[asset][Constants.HEAD].ercAmount = uint80(oraclePrice);
s.bids[asset][Constants.HEAD].creationTime = oracleTime;
}
​
//@dev Intentionally using ercAmount for oraclePrice. Storing as price may lead to bugs in the match algos.
function getPrice(address asset) internal view returns (uint80 oraclePrice) {
AppStorage storage s = appStorage();
return uint80(s.bids[asset][Constants.HEAD].ercAmount);
}
​

On shutdown LibOracle.setPriceAndTime should be called but is never called.

Impact

The above implies prices that are applied to erc redememptions are not the prices at shutdown which may lead to unfair prices, stale prices, incorrect prices for the users and protocol.
Important is that not going according to specifications can harm the reputation of the protocol/project. Its important for project to implement as intended and communicated to users.

Tools Used

Manual Analysis

Recommendations

It is recommended that when shutdown market is called, a call to LibOracle.getOraclePrice(asset) + LibOracle.setPrice(asset, oraclePrice, oracleTime) is made to save and apply the prices as at shutdown.

Updates

Lead Judging Commences

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