sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

Chinmay - Wrong method of calculating price of token1 in terms of token0 from the token0/token1 price #102

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Chinmay

high

Wrong method of calculating price of token1 in terms of token0 from the token0/token1 price

Summary

When calculating the price(conversion rate) of an underlying asset of a UNiV3 pair, it is important to consider the decimal difference between the two assets. In borrower.sol#liquidate function, the decimals are not accounted for, leading to wrong conversion rates and wrong token amounts.

Vulnerability Detail

In the liquidate function, Line 261 calculates what is the amount of token1 required to be swapped into token0 to pay for liabilities0. ```available1```` is the required amount of token1 of the pool(asset1 from v3 pool) to be swapped but the formula of calculating amount of token1 from amount of token0 ie. liabilities0 is wrong.

For example, consider the USDC/ETH pool with token0 = USDC and token1 = WETH. The correct formula to calculate price of token1 in terms of token0 is = (1 / P) * 10 *(decimal1 - decimal0) as explained here. where P is = (sqrtPriceX96 / 2^96)^2. Similarly when calculating price of token0 in terms of token1, P 10**(decimals0 - decimals1)

In our code we have just square the price without considering the possible difference in the decimals of the assets. At the time of writing, sqrtPriceX96 = 1874308838782464172402084427795214, a quick calculation of the square and dividing by 2^96 would give a value of priceX128 = 559658544 (approx.) and liabilities0 is multiplied by this big number. Instead if the correct formula would have been used, liabilities0 0.00055965854 ie. liabilities0 { P * 10 *(decimals0 - decimals1) = 559658544 10^-12 } would be the correct value.

This leads to very high estimation of the available1 because in case of USDC/ETH pool, since USDC is the less valuable token, the amount of ETH required to cover the liabilities0 amount of USDC debt should have come out as smaller because of the value of ETH, but instead it can come out as very high because we have not noted the decimal difference.

Impact

High severity because this leads to very high overestimation of token1 assets required to swap for covering the liabilities10 amount of debt, which surpasses the available inventory as well as incentive1 for liquidators. This will either fail on the next line while transfering such a big amount, or if such an amount is available in the contract(in case the borrower held very large positions as the collaetral : which is then withdrawn during liquidation), it may lead to a large loss for the borrower because only liabilities0 is expected from the swap and repaid, while sending the whole large amount available1 to the callee at Line 265. Similar problem exists in the else clause and the same problem exists when checking amounts of assets and liabilities in BalanceSheet.isHealthy function

Code Snippet

https://github.com/sherlock-audit/2023-10-aloe/blob/b60c21af24738d517941f18f7caa8c7272f771c5/aloe-ii/core/src/Borrower.sol#L261

Tool used

Manual Review

Recommendation

Correctly use the formula considering the possible difference in decimals of token0 and token1. See the linked video for details.

sherlock-admin2 commented 1 year ago

3 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, because decimals should be used to find "human readable" price, but for uniswap calculations decimals are irrelevant

tsvetanovv commented:

I think the formula is OK

MohammedRizwan commented:

valid