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

Swap fee shares the same storage variable with uplift fee leading to incorrect fee collection

goran

Summary

A high risk vulnerability exists in the UpdateWeightRunner contract where both swap fee and uplift fee management functions use the same storage variable quantAMMSwapFeeTake, causing one fee to override the other.

Vulnerability Details

The UpdateWeightRunner stores config for 2 types of fees - swap fee and uplift fee.
Admin can set the swap fee:

function setQuantAMMSwapFeeTake(
uint256 _quantAMMSwapFeeTake
) external override {
require(msg.sender == quantammAdmin, 'ONLYADMIN');
require(_quantAMMSwapFeeTake <= 1e18, 'Swap fee must be less than 100%');
uint256 oldSwapFee = quantAMMSwapFeeTake;
quantAMMSwapFeeTake = _quantAMMSwapFeeTake;
​
emit SwapFeeTakeSet(oldSwapFee, _quantAMMSwapFeeTake);
}

But there is a bug in uplift fee setter. It writes to the same storage variable quantAMMSwapFeeTake which holds info about swap fee:

/// @notice Set the quantAMM uplift fee % amount allocated to the protocol for running costs
/// @param _quantAMMUpliftFeeTake The new uplift fee % amount allocated to the protocol for running costs
function setQuantAMMUpliftFeeTake(uint256 _quantAMMUpliftFeeTake) external {
require(msg.sender == quantammAdmin, 'ONLYADMIN');
require(_quantAMMUpliftFeeTake <= 1e18, 'Uplift fee must be less than 100%');
uint256 oldSwapFee = quantAMMSwapFeeTake;
// @audit writing to the wrong storage var
quantAMMSwapFeeTake = _quantAMMUpliftFeeTake;
​
emit UpliftFeeTakeSet(oldSwapFee, _quantAMMUpliftFeeTake);
}

When either fee is updated by admin it overwrites the other fee value. Thus one of fees will always have the wrong value.

For example, admin calls setQuantAMMUpliftFeeTake(0.5e18) followed by setQuantAMMSwapFeeTake(0.1e18). Uplift fee gets overwritten and its value is 5x lower than what it is supposed to be. In we look at UplifOnlyExample.sol integration, admin will be at significant financial loss due to collecting less fees than what it is supopsed to be.

Impact

Protocol will either overcharge or undercharge fees, causing financial loss either for users or admin/protocol.

Tools Used

Manual code review

Recommendations

Introduce new storage variable for storing uplift fee and update uplift fee getter and setter to use it:

/// @notice The % of the total swap fee that is allocated to the protocol for running costs.
uint256 public quantAMMSwapFeeTake = 0.5e18;
+ uint256 public quantAMMUpliftFeeTake;
Updates

Lead Judging Commences

n0kto Lead Judge 11 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!