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

division before multiplication in `lpTokenDepositValueChange` causes loss of fees

0xhuntoor

Summary

in UpliftOnlyExample::onAfterRemoveLiquidity There is a division before multiplication in lpTokenDepositValueChange usage logic, causing any change of deposit value below 100% truncates to 0, and any change that is not perfect % of lpTokenDepositValue will have the remainder truncated

Vulnerability Details

in UpliftOnlyExample::onAfterRemoveLiquidity Hook, there is a calculation to check for lpTokenDepositValue during withdrawals to see if any uplift happened, according to that, its decided to use upliftFeeBps or minWithdrawalFeeBps

File: UpliftOnlyExample.sol
474: localData.lpTokenDepositValueChange =
475: (int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) /
476: int256(localData.lpTokenDepositValue);
477:
478: uint256 feePerLP;
​
480: if (localData.lpTokenDepositValueChange > 0) {
481: feePerLP =
482: (uint256(localData.lpTokenDepositValueChange) * (uint256(feeDataArray[i].upliftFeeBps) * 1e18)) /
483: 10000;
484: }
​
486: else {
​
489: feePerLP = (uint256(minWithdrawalFeeBps) * 1e18) / 10000;
490: }

But as you see in Line 475, we get the ratio of change by dividing by the old lpTokenDepositValue before we multiply this ration by 1e18 in Line 482

Here is an example of what would happen

  • lpTokenDepositValueNow = 15e18

  • lpTokenDepositValue = 10e18
    lpTokenDepositValueNow - lpTokenDepositValue = 15e18 - 10e18 = 5e18
    5e18 / lpTokenDepositValue = 5e18 / 10e18 = 0

lpTokenDepositValueChange = 0

Then the logic in else block will be the one to be executed, since the line 480 if evaluate to false

Also, if the uplift happened was 350%, then the 50% will truncate (35e18 - 10e18 / 10e18 = 3e18)

Impact

Loss of fees (funds) for LP providers and Quant AMM admin

Tools Used

Manual Review

Recommendations

multiply during lpTokenDepositValueChange calculation first (as seen below)

File: UpliftOnlyExample.sol
474: localData.lpTokenDepositValueChange =
475: (int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue) * 1e18) /
476: int256(localData.lpTokenDepositValue);
477:
478: uint256 feePerLP;
479: // if the pool has increased in value since the deposit, the fee is calculated based on the deposit value
480: if (localData.lpTokenDepositValueChange > 0) {
481: feePerLP =
482: (uint256(localData.lpTokenDepositValueChange) * (uint256(feeDataArray[i].upliftFeeBps))) /
483: 10000;
484: }
485: // if the pool has decreased in value since the deposit, the fee is calculated based on the base value - see wp
486: else {
487: //in most cases this should be a normal swap fee amount.
488: //there always myst be at least the swap fee amount to avoid deposit/withdraw attack surgace.
489: feePerLP = (uint256(minWithdrawalFeeBps) * 1e18) / 10000;
490: }
Updates

Lead Judging Commences

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

finding_onAfterRemoveLiquidity_lpTokenDepositValueChange_rounding_error_100%_minimum

Likelihood: High, every call to the function (withdraw) Impact: Low/Medium, uplift fees will be applied only when the price of one asset is doubled but fixed fees will still be collected.

Support

FAQs

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