hats-finance / Convergence-Finance---IBO-0x0e410e7af8e70fc5bffcdbfbdf1673ee7b3d0777

IBO, Vesting & Bond mecanism repo prepared for Hat finance audit competition
0 stars 0 forks source link

Incorrect UniV2 price oracle for pools with different token decimals #17

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @bahurum Submission hash (on-chain): 0x7ede3495d562581e385efabb22d3effb5a1af1814ee65563eba6787760ba2988 Severity: medium

Description: Description\ _getV2Price() in CvgOracle can return an incorrect price if the tokens in the pool do not have the same decimals.

Attack Scenario\ Consider a WBTC/USDC pool, where WBTC is token0. If the price of BTC is 25_000 USD, then the reserves will be

reserves WBTC = 1e8

reserves USDC = 25_000 * 1e6

reserve0 < reserve1, so price = (25_000 1e6 / 1e8) (1e16) = 2.5

running the coded PoC attached returns:

  BTC price: 2589619085176319273

which confirms the issue.

This will make the oracle call fail when checking against the chainlink price of the asset.

Recommendation Normalize the reserves to 18 decimals before dividing to obtain the price:

...
    (uint112 reserve0, uint112 reserve1, ) = IUniswapV2Pair(uniswapV2Pool).getReserves();
-   uint256 price = reserve0 < reserve1
-       ? (reserve1 * 10 ** _getDecimalsDelta(uniswapV2Pool)) / reserve0
-       : (reserve0 * 10 ** _getDecimalsDelta(uniswapV2Pool)) / reserve1;
+   reserve0 = reserve0 * 10**(18 - IERC20(IUniswapV2Pair(uniswapV2Pool).token0()).decimals());
+   reserve1 = reserve1 * 10**(18 - IERC20(IUniswapV2Pair(uniswapV2Pool).token1()).decimals());
+   uint256 price = reserve1 * 10**18 / reserve0;
    return _postTreatmentAndVerifyEth(price, isReversed, isEthPriceRelated);
...

Attachments

  1. Proof of Concept (PoC) File

Files:

bahurum commented 1 year ago

I apologize, there is a typo in the hand calculation, it should be: price = (25_000 1e6 / 1e8) (1e16) = 2.5 * 1e18

0xR3vert commented 1 year ago

Hello, Thanks a lot for your attention. You are absolutely right, we miscalculated the Univ2 price... That's a good catch, so thanks a lot for this finding. It will be very unlikely that an exploiter use this miscalculation to have a better discount on our bonds...even if the delta check will protect us from that. In conclusion we agree with you that this finding is a MEDIUM, congrats.