Using `want` in `getBeefyPrice` will cause the calculations to get rigged

[hats-bug-reporter[bot]]

Github username: -- Submission hash (on-chain): 0x1205903146dbf337279ca88ecf2e6533a7b9946e73a842e27275b5d32355276a Severity: high severity

Description:

Summary

In the function getBeefyPrice() we are using getAssetPrice(beefyVault.want()); to get the price of underlying asset but instead of underlying asset we will actually get the Price of LP tokens.

Description

function getYearnPrice(address asset) internal returns (uint256){
        IYearnToken yearnVault = IYearnToken(asset);
        uint256 underlyingPrice = getAssetPrice(yearnVault.token()); //getAssetPrice() will always have 18 decimals for Aave and Curve tokens (prices in eth), or 8 decimals if prices in USD
        //note: pricePerShare has decimals equal to underlying tokens (ex: yvUSDC has 6 decimals). By dividing by 10**yearnVault.decimals(), we keep the decimals of underlyingPrice which is 18 decimals for eth base, 8 for usd base.
        uint256 price = yearnVault.pricePerShare()*underlyingPrice / 10**yearnVault.decimals();
        if(price == 0){
            return _fallbackOracle.getAssetPrice(asset);
        }
        return price;
    }

    /**
     * @dev Gets an asset price for a beefy token
     * @param asset The asset address
     **/
    function getBeefyPrice(address asset) internal returns (uint256) {
        IBeefyVault beefyVault = IBeefyVault(asset);
        uint256 underlyingPrice = getAssetPrice(beefyVault.want().token0());
        uint256 price = beefyVault.getPricePerFullShare()*underlyingPrice / 10**beefyVault.decimals();
        if(price == 0){
            return _fallbackOracle.getAssetPrice(asset);
        }
        return price;
    }

In the function getYearnPrice() we are using getAssetPrice(yearnVault.token()) and are getting the correct price of underlying asset but in the function getBeefyPrice() instead of the underlying asset we are actually getting the price of LP tokens and it is because of simply using the want. you can check this behaviour in the follwing link https://bscscan.com/address/0x4Ea9af68bC24d14935460bb6E6d39ae11bb0Ed99#readContract

Recommendation

If we want to get the price of underlying token then instead of this line

        uint256 underlyingPrice = getAssetPrice(beefyVault.want());

we have to change it to

uint256 underlyingPrice = getAssetPrice(beefyVault.want().token0());

OR 

uint256 underlyingPrice = getAssetPrice(beefyVault.want().token1());

`want` have two tokens so if we want to get the price of token0 then we have to use first recommendation and if we want to use price of token1 then use 2nd recommendation and if want to have the price of both then use `tokens[i]` and get the price of both in ith index

[ksyao2002]

We want the price of LP tokens. If the beefy vault is yield aggregating an LP token, the payouts will also be in that LP token, so the fair pricing is to get the price of the LP token as the underlying. See: https://app.beefy.com/vault/velodrome-wsteth-weth. Quote: "Earned token is swapped for wstETH and ETH in order to acquire more of the same LP token." Also, the link you provided is for BNB chain

[Nabeel-javaid]

We want the price of LP tokens. If the beefy vault is yield aggregating an LP token, the payouts will also be in that LP token, so the fair pricing is to get the price of the LP token as the underlying. See: https://app.beefy.com/vault/velodrome-wsteth-weth. Quote: "Earned token is swapped for wstETH and ETH in order to acquire more of the same LP token." Also, the link you provided is for BNB chain

If you are taking LP in getBeefyPrice then pls change the variable name and if that's the case then you want to use price of LP in getYearnPrice function, but that's not the case, its getting underlying

[ksyao2002]

In both cases it gets the price of the underlying. If the vault is for say USDC, it will get the price of USDC. If the vault is for a LP, it will get the price of the LP. There is no issue

[Nabeel-javaid]

In both cases it gets the price of the underlying. If the vault is for say USDC, it will get the price of USDC. If the vault is for a LP, it will get the price of the LP. There is no issue

Please make it clear that are you referring to LP price or underlying price. Because the variable names are underlying and not everyone oracle have price of every token

[ksyao2002]

The underlying could be LP too. Underlying just means what you deposit to the yearn vault to earn rewards