Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

obront - LP tokens cannot be valued because ICHI cannot be priced by oracle, causing all new open positions to revert #152

Open
github-actions bot opened this issue Mar 1, 2023 · 6 comments
Labels
Escalation Resolved This issue's escalations have been approved/rejected Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@github-actions
Copy link

github-actions bot commented Mar 1, 2023

obront

high

LP tokens cannot be valued because ICHI cannot be priced by oracle, causing all new open positions to revert

Summary

In order to value ICHI LP tokens, the oracle uses the Fair LP Pricing technique, which uses the prices of both individual tokens, along with the quantities, to calculate the LP token value. However, this process requires the underlying token prices to be accessible by the oracle. Both Chainlink and Band do not support the ICHI token, so the function will fail, causing all new positions using the IchiVaultSpell to revert.

Vulnerability Detail

When a new Ichi position is opened, the ICHI LP tokens are posted as collateral. Their value is assessed using the IchiLpOracle#getPrice() function:

function getPrice(address token) external view override returns (uint256) {
    IICHIVault vault = IICHIVault(token);
    uint256 totalSupply = vault.totalSupply();
    if (totalSupply == 0) return 0;

    address token0 = vault.token0();
    address token1 = vault.token1();

    (uint256 r0, uint256 r1) = vault.getTotalAmounts();
    uint256 px0 = base.getPrice(address(token0));
    uint256 px1 = base.getPrice(address(token1));
    uint256 t0Decimal = IERC20Metadata(token0).decimals();
    uint256 t1Decimal = IERC20Metadata(token1).decimals();

    uint256 totalReserve = (r0 * px0) /
        10**t0Decimal +
        (r1 * px1) /
        10**t1Decimal;

    return (totalReserve * 1e18) / totalSupply;
}

This function uses the "Fair LP Pricing" formula, made famous by Alpha Homora. To simplify, this uses an oracle to get the prices of both underlying tokens, and then calculates the LP price based on these values and the reserves.

However, this process requires that we have a functioning oracle for the underlying tokens. However, Chainlink and Band both do not support the ICHI token (see the links for their comprehensive lists of data feeds). As a result, the call to base.getPrice(token0) will fail.

All prices are calculated in the isLiquidatable() check at the end of the execute() function. As a result, any attempt to open a new ICHI position and post the LP tokens as collateral (which happens in both openPosition() and openPositionFarm()) will revert.

Impact

All new positions opened using the IchiVaultSpell will revert when they attempt to look up the LP token price, rendering the protocol useless.

Code Snippet

https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/oracle/IchiLpOracle.sol#L19-L39

Tool used

Manual Review

Recommendation

There will need to be an alternate form of oracle that can price the ICHI token. The best way to accomplish this is likely to use a TWAP of the price on an AMM.

@github-actions github-actions bot added the High A valid High severity issue label Mar 1, 2023
@Gornutz Gornutz added Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed labels Mar 8, 2023
@Gornutz
Copy link

Gornutz commented Mar 8, 2023

There is additional oracles for assets not supported by chainlink or band but just not in scope of this specific audit.

@hrishibhat
Copy link
Contributor

Based on the context there are no implementations for getting the price of the ICHI token. Considering this a valid issue.

@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Mar 19, 2023
@SergeKireev
Copy link

SergeKireev commented Mar 20, 2023

Escalate for 31 USDC

Impact stated is medium, since positions cannot be opened and no funds are at risk.
The high severity definition as stated per Sherlock docs:

This vulnerability would result in a material loss of funds and the cost of the attack is low (relative to the amount of funds lost). The attack path is possible with reasonable assumptions that mimic on-chain conditions. The vulnerability must be something that is not considered an acceptable risk by a reasonable protocol team.

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 20, 2023

Escalate for 31 USDC

Impact stated is medium, since positions cannot be opened and no funds are at risk.
The high severity definition as stated per Sherlock docs:

This vulnerability would result in a material loss of funds and the cost of the attack is low (relative to the amount of funds lost). The attack path is possible with reasonable assumptions that mimic on-chain conditions. The vulnerability must be something that is not considered an acceptable risk by a reasonable protocol team.

You've created a valid escalation for 31 USDC!

To remove the escalation from consideration: Delete your comment.
To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin sherlock-admin added the Escalated This issue contains a pending escalation label Mar 20, 2023
@hrishibhat
Copy link
Contributor

Escalation accepted

This is a valid medium
Also Given that this is an issue only for the Ichi tokens and impact is only unable to open positions.

@sherlock-admin
Copy link
Contributor

Escalation accepted

This is a valid medium
Also Given that this is an issue only for the Ichi tokens and impact is only unable to open positions.

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.

@sherlock-admin sherlock-admin added Escalation Resolved This issue's escalations have been approved/rejected and removed Escalated This issue contains a pending escalation labels Mar 29, 2023
@hrishibhat hrishibhat added Medium A valid Medium severity issue Escalated This issue contains a pending escalation Escalation Resolved This issue's escalations have been approved/rejected and removed High A valid High severity issue Escalation Resolved This issue's escalations have been approved/rejected Escalated This issue contains a pending escalation labels Mar 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Escalation Resolved This issue's escalations have been approved/rejected Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

4 participants