Velodrome LP share price calculation is scaled incorrectly allowing users to borrow exponentially more than intended

[hats-bug-reporter[bot]]

Github username: @abhishekvispute Submission hash (on-chain): 0xc472cabb8cc02749192f09fb251a5c9314699a66054d6fd1fd5a4225e4781919 Severity: high severity

Description: Description\ VMEX has a dedicated contract called VMEXOracle.sol, serving as an entry point for all Oracle prices. Throughout the lending pool code, VMEX makes an assumption that all prices returned from VMEX oracle are of the same denomination (same decimals). It could be in ETH with 18 decimals or it could be in USD with 8 decimals. But for a given network price has to be the same no of decimals for all assets.

For example

GenericLogic.sol

calculateUserAccountData()
// this method is used to determine the borrowing capacity of user

From L225

vars.reserveUnitPrice = IPriceOracleGetter(vars.oracle)
                .getAssetPrice(vars.currentReserveAddress);

    ------
                vars.compoundedLiquidityBalance = IERC20(
                    currentReserve.aTokenAddress
                ).balanceOf(vars.user);
                // could also be in USD if reserveUnitPrice is in USD (with 8 decimals)
                vars.liquidityBalanceETH = vars.reserveUnitPrice * vars.compoundedLiquidityBalance / vars.tokenUnit;

                vars.totalCollateralInETH = vars.totalCollateralInETH + vars.liquidityBalanceETH;
------

VMEX also comments in their VMEX oracle that asset price should always have 18 decimals if using ETH and 8 decimals if using USD.

https://github.com/VMEX-finance/vmex/blob/128929d56c71860dc8e683dd8a075f3e4eca9c90/packages/contracts/contracts/protocol/oracles/VMEXOracle.sol#L181

However, the problem is Velodrome oracle returns the price in 18 decimals even if it's using USD. Hence exponentially overvaluing Velo LP shares.

POC

VelodromeOracleMock.sol

// SPDX-License-Identifier: agpl-3.0
pragma solidity 0.8.19;

import {VelodromeOracle} from "../../protocol/oracles/VelodromeOracle.sol";

contract VelodromeOracleMock {

    function calculate_stable_lp_token_price_mock(
        uint256 total_supply,
        uint256 price0,
        uint256 price1,
        uint256 reserve0,
        uint256 reserve1,
        uint256 priceDecimals
    ) external pure returns (uint256) {
        return VelodromeOracle.calculate_stable_lp_token_price(total_supply, price0, price1,reserve0, reserve1, priceDecimals);
    }

    function get_lp_price_mock(address lp_token, uint256[] memory prices, uint256 priceDecimals, bool stable) external view returns(uint256) {
        return VelodromeOracle.get_lp_price(lp_token, prices, priceDecimals, stable);
    }
}

Fork Optimsim (For USDC-DAI Stable Pool)

import { ethers } from "ethers";
const chai = require("chai");
const { expect } = chai;
import { makeSuite } from "../test-suites/test-aave/helpers/make-suite";
import { DRE } from "../helpers/misc-utils";
import {UserAccountData} from "./interfaces/index";
import {almostEqualOrEqual} from "./helpers/almostEqual";

chai.use(function (chai: any, utils: any) {
  chai.Assertion.overwriteMethod(
    "almostEqualOrEqual",
    function (original: any) {
      return function (this: any, expected: UserAccountData) {
        const actual = <UserAccountData>this._obj;

        almostEqualOrEqual.apply(this, [expected, actual]);
      };
    }
  );
});

makeSuite(
    "general oracle test ",
    () => {
    const fs = require('fs');
    const contractGetters = require('../helpers/contracts-getters.ts');

    it("Velo LP", async () => {
      const VelodromeOracle = await DRE.ethers.getContractFactory("VelodromeOracleMock");
      const velodromeOracle = await VelodromeOracle.deploy();
      console.log(await velodromeOracle.get_lp_price_mock("0x4F7ebc19844259386DBdDB7b2eB759eeFc6F8353", [100006340, 99990000], 8, true));
      // Output: BigNumber { value: "200006586833966" }

    });
    }
)

Attack Scenario\

1. Deposit Velo LP Shares
2. Borrow Exponentially More

Recommendation\ Consider downscaling VELO LP shares price to match base currency decimals

[0xcuriousapple]

You can not even mitigate this by increasing decimals inside asset mappings. For example, using decimals 10 (12 + 18) for velo lp share token decimals instead of 10 18 since asset mappings query ERC20 only to fetch decimals function getDecimals(address asset) external view

 function getDecimals(address asset) external view
        returns (
            uint256
        ){

        return IERC20Detailed(asset).decimals();
    }

[0xcuriousapple]

note: after discussion with @ksyao2002 , it turns out 1 Velo USDC-DAI LP share is worth 2m only :) didn't consider that possibility mostly false positive