code-423n4 / 2023-08-goodentry-findings

3 stars 2 forks source link

certain token pairs will be calculated and valued incorrectly, preventing compounding fees #573

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/TokenisableRange.sol#L338

Vulnerability details

Impact

The asset ratio calculation in returnExpectedBalanceWithoutFees may round to 0 with tokens that have a substantial price difference, returning incorrect values which may prevent the case for fees to be accumulated

Proof of Concept

    (amt0, amt1) = LiquidityAmounts.getAmountsForLiquidity( uint160( sqrt( (2 ** 192 * ((TOKEN0_PRICE * 10 ** TOKEN1.decimals) / TOKEN1_PRICE)) / ( 10 ** TOKEN0.decimals ) ) ), TickMath.getSqrtRatioAtTick(lowerTick), TickMath.getSqrtRatioAtTick(upperTick),  liquidity);

This function calculates sqrtPriceX96, using square root of token0Price and token1Price, corrected for difference in decimals.

The issue is caused by the inner function (2 ** 192 * ((TOKEN0_PRICE * 10 ** TOKEN1.decimals) / TOKEN1_PRICE)) / (10 ** TOKEN0.decimals) which may round to 0 making the whole expression 0, which incorrectly represents the expected value.

If this is the case then when calling claimFees() the condition which compounds the fees will never be reached

(uint256 bal0, uint256 bal1) = returnExpectedBalanceWithoutFees(0, 0);

    // If accumulated more than 1% worth of fees, compound by adding fees to Uniswap position
    if ((fee0 * 100 > bal0 ) && (fee1 * 100 > bal1)) { 

Tools Used

manual

Recommended Mitigation Steps

uint160( sqrt( (2 ** 192 * TOKEN0_PRICE) / ( TOKEN1_PRICE / 10 ** (TOKEN1.decimals - TOKEN0.decimals) ) ) ) to prevent rounding to 0

Assessed type

Math

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #106

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #316

c4-pre-sort commented 1 year ago

141345 marked the issue as not a duplicate

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #162

c4-judge commented 1 year ago

gzeon-c4 changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

gzeon-c4 marked the issue as grade-b