QuantAMM

QuantAMM
49,600 OP
View results
Submission Details
Severity: medium
Valid

The `feePerLP` can exceed 100%

Summary

The identified logical vulnerability lies in the onAfterRemoveLiquidity function, specifically around the calculation of the feePerLP variable and the potential for it to exceed 100%.

Vulnerability Details

In UpliftOnlyExample contract, the onAfterRemoveLiquidity function will charge LP fees:

localData.lpTokenDepositValue = feeDataArray[i].lpTokenDepositValue;
localData.lpTokenDepositValueChange =
(int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) /
int256(localData.lpTokenDepositValue);
uint256 feePerLP;
// if the pool has increased in value since the deposit, the fee is calculated based on the deposit value
if (localData.lpTokenDepositValueChange > 0) {
feePerLP =
(uint256(localData.lpTokenDepositValueChange) * (uint256(feeDataArray[i].upliftFeeBps) * 1e18)) /
10000;
}
// if the pool has decreased in value since the deposit, the fee is calculated based on the base value - see wp
else {
//in most cases this should be a normal swap fee amount.
//there always myst be at least the swap fee amount to avoid deposit/withdraw attack surgace.
feePerLP = (uint256(minWithdrawalFeeBps) * 1e18) / 10000;
}

The way the condition checks for localData.lpTokenDepositValueChange results in a mathematically possible situation where the fee could exceed the equity held in the liquidity token, effectively allowing for "negative" fee scenarios. When feePerLP exceeds its proper bounds, it can lead to disproportionate fees being deducted from user withdrawals.

The following line contains the logic that allows this exploit:

feePerLP =
(uint256(localData.lpTokenDepositValueChange) * (uint256(feeDataArray[i].upliftFeeBps) * 1e18)) /
10000;

If localData.lpTokenDepositValueChange is significantly high due to improper value calculations, this could cause feePerLP to surpass localData.amountLeft or even become an unintended negative value, resulting in losses for users.

Impact

The impact is HIGH and the likelihood is LOW, so the severity is MEDIUM

Tools Used

Manual Review

Recommendations

To fix the identified logical vulnerability in the onAfterRemoveLiquidity function, we need to ensure that the computed feePerLP does not exceed 100% of the total deposit being withdrawn. This can be done by implementing appropriate checks and constraints to cap the feePerLP calculation and ensure it remains within reasonable bounds.

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_onAfterRemoveLiquidity_feePerLP_no_max_can_reach_100%

Likelihood: Low, will happen only if benefits are huge. (x50 to take 100%, x25 to take 50%). Impact: feePerLP will indefinitely increase, taking users profits and finaly DoS the function.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.