QuantAMM

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

Incorrect Fee Storage Assignment Leads to Protocol-Wide Fee Miscalculation

Summary

The UpdateWeightRunner contract contains a critical vulnerability where the setQuantAMMUpliftFeeTake function incorrectly modifies the swap fee rate instead of the uplift fee rate. This misassignment leads to incorrect fee calculations throughout the protocol, potentially causing significant financial impact through improper fee collection on trades.

Vulnerability Details

In the UpdateWeightRunner contract, the setQuantAMMUpliftFeeTake function is designed to update the uplift fee percentage. However, the function erroneously updates the swap fee storage variable instead. This occurs because the function assigns the new uplift fee value to quantAMMSwapFeeTake, which is the storage variable meant for swap fees.

The vulnerable code:

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; // Incorrect storage variable
emit UpliftFeeTakeSet(oldSwapFee, _quantAMMUpliftFeeTake);
}

Scenario: Consider the following sequence of events:

  1. The protocol initially has a swap fee set to 0.5% (0.005e18)

  2. The protocol administrator intends to set a new uplift fee of 1% (0.01e18)

  3. The administrator calls setQuantAMMUpliftFeeTake(0.01e18)

  4. Instead of setting the uplift fee, the function changes the swap fee to 1%

  5. All subsequent trades now use double the intended swap fee

  6. The uplift fee remains undefined or uses an uninitialized value

Impact

The vulnerability has severe economic implications:

  1. All trades in the protocol use incorrect fee calculations

  2. Users pay more or less in fees than intended

  3. Protocol revenue calculations become inaccurate

  4. The uplift fee mechanism becomes completely ineffective

  5. The bug compounds with every trade executed

  6. Protocol administrators lose the ability to properly manage different fee types

Tools Used

Manual code review

  • Control flow analysis

  • Economic impact analysis

  • Cross-reference analysis with protocol documentation

Recommendations

Implement proper storage for uplift fees:

contract UpdateWeightRunner {
uint256 public quantAMMSwapFeeTake = 0.5e18;
uint256 public quantAMMUpliftFeeTake = 0.5e18; // Add separate storage

function setQuantAMMUpliftFeeTake(uint256 _quantAMMUpliftFeeTake) external {
require(msg.sender == quantammAdmin, "ONLYADMIN");
require(_quantAMMUpliftFeeTake <= 1e18, "Uplift fee must be less than 100%");
uint256 oldUpliftFee = quantAMMUpliftFeeTake; // Use correct storage
quantAMMUpliftFeeTake = _quantAMMUpliftFeeTake; // Update correct variable
emit UpliftFeeTakeSet(oldUpliftFee, _quantAMMUpliftFeeTake);
}
function getQuantAMMUpliftFeeTake() external view returns (uint256) {
return quantAMMUpliftFeeTake; // Add getter for uplift fee
}

}

Add proper validation and documentation:

  • Document the expected ranges and uses for each fee type

  • Add invariant checks to ensure fees remain within acceptable bounds

  • Implement events that clearly distinguish between fee types

  • Add function comments explaining the purpose of each fee

  • Consider implementing a fee management system:

    • Create a dedicated fee management contract

    • Implement role-based access control for fee modifications

    • Add time-locks for fee changes

    • Include emergency pause functionality for fee collection

  • Add comprehensive testing:

    • Unit tests for each fee modification function

    • Integration tests for fee calculations

    • Scenario tests for various fee combinations

    • Fuzz testing for fee boundaries

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!