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

0 stars 0 forks source link

H-02 MitigationConfirmed #34

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

Vulnerability details

Comments

The current getTVL only calculates the token value of TokenisableRange, ignoring the value of token0 and token1, which leads to two problems:

  1. The users who deposit tokens and increaseLiquidity with imperfect proportions will suffer losses. The getTVL is underestimated and excess token0 and token1 are stuck in the GeVault contract, cannot be withdraw.
  2. When isEnabled is false, withdraw will removeFromAllTicks but will not deployAssets, which causes getTVL to be 0 (because token0 and token1's value are not calculated), causing other users except the first user to be unable to withdraw.

Mitigation

    valueX8 = token0.balanceOf(address(this)) * oracle.getAssetPrice(address(token0)) / 10**token0.decimals();
    valueX8 += token1.balanceOf(address(this)) * oracle.getAssetPrice(address(token1)) / 10**token1.decimals();

The above-mentioned problems can be perfectly solved by calculating the values ​​of token0 and token1 in getTVL:

  1. For excess tokens, tokens are minted according to the input value when deposit, and tokens are withdrawn in proportion to the value when withdraw.
  2. When isEnabled is false, removeFromAllTicks only occur at first time, and then withdraw will directly allocate token0 and token1 in value.

In addition, getTVL was reconstructed through two PRs:

The first PR fixed the problem, and the second PR introduced a bug, which is different from the root cause of the original issue, so I submitted it as a new issue.

Conclusion

LGTM

c4-judge commented 12 months ago

gzeon-c4 marked the issue as confirmed for report

c4-judge commented 12 months ago

gzeon-c4 marked the issue as satisfactory