Can proposeRedemption based on an outdated price

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/RedemptionFacet.sol#L75

Vulnerability details

Impact

If the collateral ratio is calculated based on an incorrect price, a ShortRecord that is not a target for redemption at the current price can be proposed.

Proof of Concept

It uses the cached price for calculating the collateral ratio in proposeRedemption. It uses it without checking whether the cached price information is outdated. If the collateral ratio is calculated based on an incorrect price, a ShortRecord that is not a target for redemption at the current price can be proposed.

function proposeRedemption(
    address asset,
    MTypes.ProposalInput[] calldata proposalInput,
    uint88 redemptionAmount,
    uint88 maxRedemptionFee
) external isNotFrozen(asset) nonReentrant {
    if (proposalInput.length > type(uint8).max) revert Errors.TooManyProposals();
    MTypes.ProposeRedemption memory p;
    p.asset = asset;
    STypes.AssetUser storage redeemerAssetUser = s.assetUser[p.asset][msg.sender];
    uint256 minShortErc = LibAsset.minShortErc(p.asset);

    if (redemptionAmount < minShortErc) revert Errors.RedemptionUnderMinShortErc();

    if (redeemerAssetUser.ercEscrowed < redemptionAmount) revert Errors.InsufficientERCEscrowed();

    // @dev redeemerAssetUser.SSTORE2Pointer gets reset to address(0) after actual redemption
    if (redeemerAssetUser.SSTORE2Pointer != address(0)) revert Errors.ExistingProposedRedemptions();

@>  p.oraclePrice = LibOracle.getPrice(p.asset); // no price update before use this cached price

    bytes memory slate;
    for (uint8 i = 0; i < proposalInput.length; i++) {
        ...
@>      p.currentCR = currentSR.getCollateralRatio(p.oraclePrice);

        ...
@>      p.colRedeemed = p.oraclePrice.mulU88(p.amountProposed);
        if (p.colRedeemed > currentSR.collateral) {
            p.colRedeemed = currentSR.collateral;
        }
        ...
    }
    ...
}

Tools Used

Manual Review

Recommended Mitigation Steps

function proposeRedemption(
    address asset,
    MTypes.ProposalInput[] calldata proposalInput,
    uint88 redemptionAmount,
    uint88 maxRedemptionFee
) external isNotFrozen(asset) nonReentrant {
    if (proposalInput.length > type(uint8).max) revert Errors.TooManyProposals();
    MTypes.ProposeRedemption memory p;
    p.asset = asset;
    STypes.AssetUser storage redeemerAssetUser = s.assetUser[p.asset][msg.sender];
    uint256 minShortErc = LibAsset.minShortErc(p.asset);

    if (redemptionAmount < minShortErc) revert Errors.RedemptionUnderMinShortErc();

    if (redeemerAssetUser.ercEscrowed < redemptionAmount) revert Errors.InsufficientERCEscrowed();

    // @dev redeemerAssetUser.SSTORE2Pointer gets reset to address(0) after actual redemption
    if (redeemerAssetUser.SSTORE2Pointer != address(0)) revert Errors.ExistingProposedRedemptions();

-    p.oraclePrice = LibOracle.getPrice(p.asset);
+    p.oraclePrice = LibOracle.getSavedOrSpotOraclePrice(p.asset);
    ...
}

Assessed type

Oracle

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #114

[raymondfam]

See #128.

[c4-judge]

hansfriese marked the issue as satisfactory