QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: low
Invalid

quantAMMSwapFeeTake and setQuantAMMUpliftFeeTake Can Be Set to 100%, Allowing Admin to Mistakenly Disrupt Protocol Functionality

theirrationalone

Summary

In the UpdateWeightRunner contract, there are functions called setQuantAMMSwapFeeTake and setQuantAMMUpliftFeeTake, which are protected by admin-only access control. While functions performs its intended job correctly, there is a critical oversight: an admin could mistakenly set the quantAMMSwapFeeTake and quantAMMUpliftFeeTake value to 100% aka 1e18. This creates the potential for unintended disruption of the protocol's functionality, as there is no validation check to prevent the admin from setting this parameter to 100%.

This vulnerability highlights the importance of implementing input validation, even for admin-controlled functions, to avoid accidental misconfiguration that could harm the protocol's usability.

Vulnerability Details

Code Snippet

Below is the relevant code from UpdateWeightRunner::setQuantAMMSwapFeeTake:

function setQuantAMMSwapFeeTake(uint256 _quantAMMSwapFeeTake) external override {
require(msg.sender == quantammAdmin, "ONLYADMIN");
@> // @info: _quantAMMSwapFeeTake should be less than 1e18 (100%). The condition should be _quantAMMSwapFeeTake < 1e18
@> require(_quantAMMSwapFeeTake <= 1e18, "Swap fee must be less than 100%");
-----------------------------------^
uint256 oldSwapFee = quantAMMSwapFeeTake;
quantAMMSwapFeeTake = _quantAMMSwapFeeTake;
​
emit SwapFeeTakeSet(oldSwapFee, _quantAMMSwapFeeTake);
}

Below is the relevant code from UpdateWeightRunner::setQuantAMMUpliftFeeTake:

function setQuantAMMUpliftFeeTake(uint256 _quantAMMUpliftFeeTake) external {
require(msg.sender == quantammAdmin, "ONLYADMIN");
@> // @info: _quantAMMUpliftFeeTake should be less than 1e18 aka 100%, therefore the condition should be _quantAMMUpliftFeeTake < 1e18
@> require(_quantAMMUpliftFeeTake <= 1e18, "Uplift fee must be less than 100%");
----------------------------------------^
uint256 oldSwapFee = quantAMMSwapFeeTake;
quantAMMSwapFeeTake = _quantAMMUpliftFeeTake;
​
emit UpliftFeeTakeSet(oldSwapFee, _quantAMMUpliftFeeTake);
}

Explanation

The require statements currently allow the quantAMMSwapFeeTake and quantAMMUpliftFeeTake parameters to be set to 100% (1e18). While this may seem harmless during standard operation, an accidental or incorrect input by the admin could result in severe consequences for the protocol's functionality.

Without a restriction to ensure the parameter remains strictly less than 100%, the oversight could allow:

  1. The fee for swaps and uplifts to be set to an exorbitant level, effectively halting user activity.

  2. Significant disruption of user experience and potential reputational damage to the protocol.

This lack of input validation represents a preventable vulnerability that could have far-reaching implications.

Impact

If the quantAMMSwapFeeTake and quantAMMUpliftFeeTake are mistakenly set to 100% 1e18, the following issues may arise:

  1. Exorbitant Fees: Users performing swaps and uplifts would have to pay 100% of of swap and uplift fees.

  2. Protocol Inaccessibility: With users discouraged from performing swaps and uplifts due to excessive fees, the protocol's core functionality could effectively be disabled.

  3. Reputational Damage: Such misconfigurations, even if accidental, could erode user trust in the protocol's reliability and security.

Tools Used

Manual Review

Recommendations

Mitigation

To address this issue, modify the require statement to ensure that the quantAMMSwapFeeTake and quantAMMUpliftFeeTake parameters are strictly less than 100%. The corrected code is shown below:

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

Rationale

This change ensures that the quantAMMSwapFeeTake and quantAMMUpliftFeeTake values are always less than 100%, preventing accidental misconfiguration by the admin. It adds a simple but effective layer of input validation that eliminates the possibility of setting a fee value that could disrupt the protocol.

By implementing this fix, the protocol can safeguard its functionality and user experience against such errors, ensuring reliable and uninterrupted operation.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas / Admin is trusted / Pool creation is trusted / User mistake / Suppositions

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelyhood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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

Give us feedback!