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

Swap fee and Uplift fee state variable confusion in `UpdateWeightRunner`

tinnohofficial

Summary

The UpdateWeightRunner contract has an issue where the uplift fee and swap fee mechanisms are incorrectly sharing the same state variable quantAMMSwapFeeTake, causing setter functions to overwrite one another when called and thus causing incorrect fees to be set.

Vulnerability Details

The contract attempts to manage two distinct fee types - swap fees and uplift fees. However, the implementation uses a single state variable quantAMMSwapFeeTake for both purposes.
As it can be seen setQuantAMMSwapFeeTake sets the quantAMMSwapFeeTake (which here is the desired behaviour)

function setQuantAMMSwapFeeTake(uint256 _quantAMMSwapFeeTake) external override {
// ... snip ...
quantAMMSwapFeeTake = _quantAMMSwapFeeTake;
// ...

But on the contrary, the function setQuantAMMUpliftFeeTake also sets the same variable quantAMMSwapFeeTake

function setQuantAMMUpliftFeeTake(uint256 _quantAMMUpliftFeeTake) external {
require(msg.sender == quantammAdmin, "ONLYADMIN");
require(_quantAMMUpliftFeeTake <= 1e18, "Uplift fee must be less than 100%");
uint256 oldSwapFee = quantAMMSwapFeeTake;
@> quantAMMSwapFeeTake = _quantAMMUpliftFeeTake; // @audit we are setting the wrong thing.
​
emit UpliftFeeTakeSet(oldSwapFee, _quantAMMUpliftFeeTake);
}

It can also be observed that both getQuantAMMUpliftFeeTake and getQuantAMMSwapFeeTake return quantAMMSwapFeeTake.

Therefore, this is problematic since the swap fee and the uplift fee are distinct;

  • Swap fee is taken as % of total swap fee that is allocated to the protocol for running costs WHILE

  • Uplift fee is taken from LP's profit when they remove liquidity from the pool.

Based on this clear distinction, the fees should therefore be separately tracked and NOT the same variable to avoid confusion.

PoC

  1. All contract deployments are properly done.

  2. Quantamm admin calls setQuantAMMUpliftFeeTake in order to set say a 10% uplift fee for the LPs when they withdraw.

  3. Quantamm also admin calls setQuantAMMSwapFeeTake in order to set say a 2% fee for the swaps.

  4. Since both swaps and uplifts use the same fees variable either the LPs (when withdrawing liquidity) or the users (when swapping) will get higher fees (incur loss) or low fees (unfair advantage) depending on which setter function and which percent the admin set.

Impact

The protocol cannot maintain different values for swap fees and uplift fees meaning that any update to uplift fees will overwrite swap fees. This makes the protocol's fee structure to be broken.

Tools Used

Manual Review

Recommendations

Add a separate state variable for uplift fee.

uint256 public quantAMMSwapFeeTake = 0.5e18;
+ uint256 public quantAMMUpliftFeeTake = 0.5e18;

Fix the setter function below.

function setQuantAMMUpliftFeeTake(uint256 _quantAMMUpliftFeeTake) external {
require(msg.sender == quantammAdmin, "ONLYADMIN");
require(_quantAMMUpliftFeeTake <= 1e18, "Uplift fee must be less than 100%");
- uint256 oldSwapFee = quantAMMSwapFeeTake;
+ uint256 oldUpliftFee = quantAMMUpliftFeeTake;
- quantAMMSwapFeeTake = _quantAMMSwapFeeTake;
+ quantAMMUpliftFeeTake = _quantAMMUpliftFeeTake;
emit UpliftFeeTakeSet(oldUpliftFee, _quantAMMUpliftFeeTake);
}

Fix the getter function below.

function getQuantAMMUpliftFeeTake() external view returns (uint256) {
- return quantAMMSwapFeeTake;
+ return quantAMMUpliftFeeTake;
}
Updates

Lead Judging Commences

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

finding_quantAMMSwapFeeTake==quantAMMUplfitFeeTake

Likelyhood: High, calling setters or getters Impact: Low/Medium, both getters return `quantAMMSwapFeeTake` and `setQuantAMMUpliftFeeTake` modify `quantAMMUplfitFeeTake`. Real impact: those 2 values will be always the same.

Support

FAQs

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

Give us feedback!