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

MinimumVarianceUpdateRule Asymmetric Handling of Negative Prices

iepathos

Summary

The MinimumVarianceUpdateRule contract handles negative prices asymmetrically compared to positive prices, leading to inconsistent weight calculations. While ChainlinkOracle enforces positive prices (require(data > 0)), negative prices can still occur through the project's MultiHopOracle which performs mathematical operations that may result in negative values. This asymmetry (4.1%) could cause significant pool imbalances in pools using MultiHopOracle or similar oracles that support negative prices.

Vulnerability Details

Location: pkg/pool-quantamm/contracts/rules/MinimumVarianceUpdateRule.sol

The issue occurs in the weight calculation where negative prices produce asymmetric results compared to equivalent positive prices:

int256 precision = ONE.div(newWeights[i]);
divisionFactor += precision;
newWeights[i] = oneMinusLambda.mul(precision);

When handling negative prices (which can occur through MultiHopOracle's mathematical operations), the sign propagates through multiple calculations affecting both the normalization factor and final weights. The asymmetry is amplified by the variance calculations.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.26;
​
import "forge-std/Test.sol";
import "../../../contracts/mock/MockRuleInvoker.sol";
import "../../../contracts/mock/mockRules/MockMinimumVarianceRule.sol";
import "../../../contracts/mock/MockPool.sol";
import "../utils.t.sol";
​
contract QuantammMinimumVarianceNegativePriceTest is Test, QuantAMMTestUtils {
MockMinimumVarianceRule internal rule;
MockPool internal mockPool;
function setUp() public {
rule = new MockMinimumVarianceRule(address(this));
mockPool = new MockPool(3600, PRBMathSD59x18.fromInt(1), address(rule));
mockPool.setNumberOfAssets(2);
}
​
function testMinimumVarianceNegativePriceSymmetry() public {
// Setup parameters
int256[][] memory parameters = new int256[][]();
parameters[0] = new int256[]();
parameters[0][0] = 0.5e18; // mixing lambda
​
int256[] memory prevWeights = new int256[]();
prevWeights[0] = 0.5e18;
prevWeights[1] = 0.5e18;
​
int256[] memory positivePrices = new int256[]();
positivePrices[0] = 2e18;
positivePrices[1] = 1e18;
​
int256[] memory negativePrices = new int256[]();
negativePrices[0] = -2e18;
negativePrices[1] = 1e18;
​
// Setup variances
int256[] memory variances = new int256[]();
variances[0] = 1e18;
variances[1] = 1e18;
​
// Setup moving averages (4 values for 2 assets)
int256[] memory prevMovingAverages = new int256[]();
prevMovingAverages[0] = 1e18;
prevMovingAverages[1] = 1e18;
prevMovingAverages[2] = 1e18;
prevMovingAverages[3] = 1e18;
​
int128[] memory lambda = new int128[]();
lambda[0] = int128(0.1e18);
​
// Initialize pool
rule.initialisePoolRuleIntermediateValues(
address(mockPool),
prevMovingAverages,
variances,
2
);
​
// Test with positive prices
rule.CalculateUnguardedWeights(
prevWeights,
positivePrices,
address(mockPool),
parameters,
lambda,
prevMovingAverages
);
int256[] memory positiveWeights = rule.GetResultWeights();
​
// Test with negative prices
rule.CalculateUnguardedWeights(
prevWeights,
negativePrices,
address(mockPool),
parameters,
lambda,
prevMovingAverages
);
int256[] memory negativeWeights = rule.GetResultWeights();
​
// Log results
emit log_named_decimal_int("Positive price weight 0", positiveWeights[0], 18);
emit log_named_decimal_int("Positive price weight 1", positiveWeights[1], 18);
emit log_named_decimal_int("Negative price weight 0", negativeWeights[0], 18);
emit log_named_decimal_int("Negative price weight 1", negativeWeights[1], 18);
​
// Validate weight ranges
require(positiveWeights[0] >= 0 && positiveWeights[0] <= 1e18, "Invalid positive weight range");
require(negativeWeights[0] >= 0 && negativeWeights[0] <= 1e18, "Invalid negative weight range");
// Check symmetry
assertApproxEqAbs(positiveWeights[0], negativeWeights[0], 1e16, "Weight asymmetry detected");
}
}

Test Results:

Positive price weight 0: 0.291666666666666666
Positive price weight 1: 0.708333333333333333
Negative price weight 0: 0.250548245614035087
Negative price weight 1: 0.749451754385964912

Note that the asymmetry (0.041 or 4.1%) is larger than AntiMomentumUpdateRule (1.7%) but smaller than DifferenceMomentumUpdateRule (8.7%). The weight difference could lead to substantial pool imbalances.

Impact

  • Significant asymmetric weight calculations for positive vs negative prices (4.1%)

  • Inconsistent pool behavior depending on price sign

  • Could lead to substantial weight distribution imbalances

  • May create arbitrage opportunities due to predictable asymmetry

  • Breaks mathematical symmetry expected in price calculations

  • Could compound with other weight calculation issues

  • Particularly concerning due to interaction with variance calculations

Recommendations

  1. Modify price handling to maintain symmetry:

// Handle sign separately from magnitude
int256 sign = locals.denominator >= 0 ? ONE : -ONE;
int256 magnitude = locals.denominator >= 0 ? locals.denominator : -locals.denominator;
locals.newWeights[locals.i] = sign.mul(ONE.div(magnitude)).mul(int256(locals.newWeights[locals.i]));

  1. Consider architectural improvements:

    • Add explicit sign handling throughout calculations

    • Implement symmetry validation in tests

    • Add invariant checks for price sign handling

    • Consider using absolute values for intermediate calculations

    • Add documentation about price sign handling expectations

    • Consider extracting common price handling logic to a shared library

    • Add specific checks for variance calculations with negative prices

  2. Add comprehensive tests:

    • Test symmetry with various price magnitudes

    • Test edge cases with extreme price values

    • Add property-based tests for price sign handling

    • Test interactions between variance and price signs

    • Test compounding effects over multiple updates

    • Test interactions with other weight calculation components

    • Add specific tests for variance-based edge cases

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

invalid_getData_negative_or_zero_price

Multihop will call ChainlinkOracle and the check is in it: `require(data > 0, "INVLDDATA");` MultiHop is just here to combine Chainlinks feed when there is no direct USD price feed for a token.

Support

FAQs

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

Give us feedback!