Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coffiasd - chainlink oracles that have different decimals will return the wrong prices #135

Closed
sherlock-admin4 opened this issue Sep 10, 2024 · 0 comments
Labels
Non-Reward This issue will not receive a payout

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Sep 10, 2024

coffiasd

Medium

chainlink oracles that have different decimals will return the wrong prices

Summary

During the liquidation process, the protocol converts the debtToCover amount into the equivalent baseCollateral amount using two prices fetched from the Chainlink data feed. However, the protocol incorrectly assumes that both assets have the same decimal, which leads to price errors when the assets have different decimals.

Root Cause

LiquidationLogic.sol::executeLiquidationCall Fetch two assets price from chainlink data feed.

    (vars.actualCollateralToLiquidate, vars.actualDebtToLiquidate, vars.liquidationProtocolFeeAmount) =
    _calculateAvailableCollateralToLiquidate(
      collateralReserve,
      vars.debtReserveCache,
      vars.actualDebtToLiquidate,
      vars.userCollateralBalance,
      vars.liquidationBonus,
      IPool(params.pool).getAssetPrice(params.collateralAsset),
      IPool(params.pool).getAssetPrice(params.debtAsset),
      IPool(params.pool).factory().liquidationProtocolFeePercentage()
    );

LiquidationLogic.sol::_calculateAvailableCollateralToLiquidate and then converts the debtToCover amount into the equivalent baseCollateral amount

    unchecked {
      vars.collateralAssetUnit = 10 ** vars.collateralDecimals;
      vars.debtAssetUnit = 10 ** vars.debtAssetDecimals;
    }

    // This is the base collateral to liquidate based on the given debt to cover
    vars.baseCollateral = ((vars.debtAssetPrice * debtToCover * vars.collateralAssetUnit)) / (vars.collateralPrice * vars.debtAssetUnit);

chainlink data feed decimals is ignored and assumes that both assets have the same decimal.

Internal pre-conditions

No response

External pre-conditions

1.two assets using different decimals

Attack Path

No response

Impact

the calculation of baseCollateral is incorrect which can lead to protocol lost of funds

PoC

ampl-usd return 18 decimals
https://data.chain.link/feeds/ethereum/mainnet/ampl-usd

eth-usd return 8 decimals
https://data.chain.link/streams/eth-usd

Mitigation

compare two assets decimals before use it

Duplicate of #166

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A High severity issue. labels Sep 20, 2024
@nevillehuang nevillehuang removed the High A High severity issue. label Oct 2, 2024
@sherlock-admin3 sherlock-admin3 changed the title Rapid Onyx Meerkat - chainlink oracles that have different decimals will return the wrong prices coffiasd - chainlink oracles that have different decimals will return the wrong prices Oct 3, 2024
@sherlock-admin3 sherlock-admin3 added Non-Reward This issue will not receive a payout and removed Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

3 participants