c4-submissions commented 1 year ago

Lines of code

Vulnerability details

Comments

As described in H-01, if the current slot is not within the tick range, the calculated token0Amount or token1Amount is 0, causing the fee calculation to be all single token. Malicious attackers or market changes can change slot0Tick, cause mint's LP to be 0 when a user deposits, and all single token is lost as a fee. Assume slot0Tick < lowerTick, token1Amount = newFee1 = n1 = newLiquidity = 0, all token1 is treated as fee, only token0 is returned.

Mitigation

feeLiquidity = newLiquidity * ( (fee0 * TOKEN0_PRICE / 10 ** TOKEN0.decimals) + (fee1 * TOKEN1_PRICE / 10 ** TOKEN1.decimals) )   
                                    / ( (added0   * TOKEN0_PRICE / 10 ** TOKEN0.decimals) + (added1   * TOKEN1_PRICE / 10 ** TOKEN1.decimals) );

Fees are now calculated by the amount of liquidity added, regardless of tick range, which effectively mitigates the above problems. By the way, the calculation formula of feeLiquidity means that the larger n0 and n1, the larger newLiquidity, added0, and added1 are, and the smaller feeLiquidity is. Generally speaking, fees and quantities increase proportionally, but here it is inversely proportional，not sure if this is intentionally designed that way.

Suggestion

The root of the problem is that the actual calculation results may not match the user's intention. It is recommended to add a new parameter lpMin in the deposit, and finally verify that lpAmt >= lpMin.

Conclusion

LGTM

c4-judge commented 12 months ago

gzeon-c4 marked the issue as satisfactory