search

code-423n4 / 2023-09-goodentry-mitigation-findings

0 stars 0 forks source link

The number of ticks is incorrectly fixed and is not equal to liquidityPerTick, resulting in low fund utilization. #54

Closed c4-submissions closed 12 months ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/GoodEntry-io/ge/blob/c7c7de57902e11e66c8186d93c5bb511b53a45b8/contracts/GeVault.sol#L54 https://github.com/GoodEntry-io/ge/blob/c7c7de57902e11e66c8186d93c5bb511b53a45b8/contracts/GeVault.sol#L344-L367

Vulnerability details

Impact

In the readme, it states the goal after activeIndex is reconstructed is: Instead of depositing half of the assets into each of the 2 ticks above and below, this has been parameterized, allowing to change asset distribution in case of high volatility. But the range of tick is still fixed as 2 above and below, the default value of liquidityPerTick is set to 3, which means that under normal conditions at least 1/3 of the funds are idle and the utilization rate is low.

Proof of Concept

    if (newTickIndex > 1) 
      depositAndStash(
        ticks[newTickIndex-2], 
        baseTokenIsToken0 ? 0 : availToken0 / liquidityPerTick,
        baseTokenIsToken0 ? availToken1 / liquidityPerTick : 0
      );
    if (newTickIndex > 0) 
      depositAndStash(
        ticks[newTickIndex-1], 
        baseTokenIsToken0 ? 0 : availToken0 / liquidityPerTick,
        baseTokenIsToken0 ? availToken1 / liquidityPerTick : 0
      );
    if (newTickIndex < ticks.length) 
      depositAndStash(
        ticks[newTickIndex], 
        baseTokenIsToken0 ? availToken0 / liquidityPerTick : 0,
        baseTokenIsToken0 ? 0 : availToken1 / liquidityPerTick
      );
    if (newTickIndex+1 < ticks.length) 
      depositAndStash(
        ticks[newTickIndex+1], 
        baseTokenIsToken0 ? availToken0 / liquidityPerTick : 0,
        baseTokenIsToken0 ? 0 : availToken1 / liquidityPerTick
      );

It is clearly seen from the code that the ticks are fixed at 4 and fluctuate between -2 and 2 around activeTick. For token0 and token1, there are only two ticks at most, but liquidityPerTick is set to 3, causing funds to be idle.

Tools Used

Manual review

Recommended Mitigation Steps

The tick range should not be fixed at 4, but should be equal to liquidityPerTick. That is, depositAndStash should be called in a loop, and the number of loops is liquidityPerTick * 2.

Assessed type

Context

gzeon-c4 commented 12 months ago

Fixed in https://github.com/GoodEntry-io/ge/pull/11

c4-judge commented 12 months ago

gzeon-c4 marked the issue as nullified