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

Incorrect Handling of Scalar instead of Vector Q Values in the `_getWeights` Function of `PowerChannelUpdateRule.sol` Contract

eta

Summary

The QuantAMMPowerChannelLocals struct and _getWeights function within the PowerChannelUpdateRule.sol contract improperly handle the distinction between scalar (q) and vector (vectorQ) values. In the current implementation, when _parameters[1] contains multiple elements (indicating a vector scenario), the code erroneously updates the scalar q instead of the vector vectorQ. This design flaw can lead to Incorrect update and query of struct, attack risks, and compatibility with other components.

Vulnerability Details

The issue arises in the _getWeights function when determining whether the q parameter is a scalar or a vector:

  • In scalar scenarios, the code correctly sets locals.q to _parameters[1][0].

  • However, in vector scenarios, instead of updating locals.vectorQ, the loop overwrites the scalar locals.q for each iteration.

  • Subsequent calculations use the incorrectly updated scalar q, leading to invalid update and query of QuantAMMPowerChannelLocals struct , when _parameters[1] is meant to represent a vector.

/// @notice w(t) = w(t − 1) + κ · ( sign(1/p(t)*∂p(t)/∂t) * |1/p(t)*∂p(t)/∂t|^q − ℓp(t) ) where ℓp(t) = 1/N * ∑(sign(1/p(t)*∂p(t)/∂t) * |1/p(t)*∂p(t)/∂t|^q)
/// @param _prevWeights the previous weights retrieved from the vault
/// @param _data the latest data from the signal, usually price
/// @param _parameters the parameters of the rule that are not lambda
/// @param _poolParameters pool parameters [0]=k, [1]=q, can be per token (vector) or single for all tokens (scalar), [2]=useRawPrice
function _getWeights(
int256[] calldata _prevWeights,
int256[] memory _data,
int256[][] calldata _parameters,
QuantAMMPoolParameters memory _poolParameters
) internal override returns (int256[] memory newWeightsConverted) {
​
...
bool scalarQ = _parameters[1].length == 1;
locals.q = _parameters[1][0];
for (locals.i = 0; locals.i < locals.prevWeightsLength; ) {
locals.denominator = _poolParameters.movingAverage[locals.i];
// @audit updates the scalar `q` instead of the vector `vectorQ`, even if it is not scalar
if(!scalarQ){
locals.q = _parameters[1][locals.i];
}
​
if (locals.useRawPrice) {
locals.denominator = _data[locals.i];
}
​
locals.intermediateRes = ONE.div(locals.denominator).mul(locals.newWeights[locals.i]);
​
unchecked {
locals.sign = locals.intermediateRes >= 0 ? ONE : -ONE;
}
//sign(1/p(t)*∂p(t)/∂t) * |1/p(t)*∂p(t)/∂t|^q
//stored as it is used in multiple places, saves on recalculation gas. _pow is quite expensive
// @audit `vectorQ` has not been used and updated in the contract
locals.newWeights[locals.i] = locals.sign.mul(_pow(locals.intermediateRes.abs(), locals.q));
​
if (locals.kappa.length == 1) {
locals.normalizationFactor += locals.newWeights[locals.i];
} else {
locals.normalizationFactor += locals.kappa[locals.i].mul(locals.newWeights[locals.i]);
}
​
unchecked {
++locals.i;
}
}
...
​

Impact

  • Incorrect update and query of the QuantAMMPowerChannelLocals struct: The current implementation contradicts the design intent of distinguishing between scalar and vector Q values.The use of locals.vectorQ is part of the design intention to handle vector scenarios. By misusing locals.q, the code violates this intention, leading to inconsistency and reducing the maintainability of the code.

  • Attack Risks: Mismanagement of parameters increases the attack surface for malicious actors, who might exploit these inconsistencies to manipulate the pool's behavior.

  • Compatibility with Other Components: The behavior deviates from the intended functionality as described in the protocol's specifications, potentially breaking compatibility with other components or strategies reliant on the expected weight update mechanics.

Tools Used

Manual Review

Recommendations

It is recommended that locals.q and locals.vectorQ be maintained as separate fields in the QuantAMMPowerChannelLocals struct, with locals.q used for scalar updates and locals.vectorQ for vector-based calculations. This approach ensures easier querying and maintenance of arrays in future updates and interaction with other programs.

locals.q = _parameters[1][0];
if(!scalarQ){
- locals.q = _parameters[1][locals.i];
+ locals.vectorQ[locals.i] = _parameters[1][locals.i];
}
​
locals.newWeights[locals.i] = locals.sign.mul(_pow(locals.intermediateRes.abs(), locals.q));
+ if(!scalarQ){
+ locals.newWeights[locals.i] = locals.sign.mul(_pow(locals.intermediateRes.abs(), locals.vectorQ[locals.i]));
+ }
​
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!