QuantAMM

49,600 OP

View results

Previous Next

Submission Details

Severity: high

Valid

In case of `minimumVariance` No `performUpdate` could be executed after first one

TheKhans

Summary

In minimum variance, the previous moving average is stored/packed. However, due to incorrect unpacking, no performUpdate is executed after the first one.

Vulnerability Details

The WeightedPool using minimum variance also stores the previous moving average. During the deployment of the weighted pool, the initial moving average is stored as part of the pool's initialization.

contracts/rules/UpdateRule.sol:247

247: function initialisePoolRuleIntermediateValues(

248: address _poolAddress,

249: int256[] memory _newMovingAverages,

250: int256[] memory _newInitialValues,

251: uint _numberOfAssets

252: ) external {

253: //initialisation is controlled during the registration process

254: //this is to make sure no external actor can call this function

255: require(msg.sender == _poolAddress || msg.sender == updateWeightRunner, "UNAUTH");

256: _setInitialMovingAverages(_poolAddress, _newMovingAverages, _numberOfAssets);

257: _setInitialIntermediateValues(_poolAddress, _newInitialValues, _numberOfAssets);

258: }

259:

Here, at line 256, the _setInitialMovingAverages function is called.

contracts/rules/base/QuantammMathMovingAverage.sol:67

67: function _setInitialMovingAverages(

68: address _poolAddress,

69: int256[] memory _initialMovingAverages,

70: uint _numberOfAssets

71: ) internal {

72: uint movingAverageLength = movingAverages[_poolAddress].length;

73:

74: if (movingAverageLength == 0 || _initialMovingAverages.length == _numberOfAssets) {

75: //should be during create pool

76: movingAverages[_poolAddress] = _quantAMMPack128Array(_initialMovingAverages);

77: } else {

78: revert("Invalid set moving avg");

79: }

80: }

The movingAverage values are packed into a 128 array and stored in movingAverages[_poolAddress]. These values are fetched when the performUpdate function is called, which in turn invokes CalculateNewWeights to determine the new weights for the weighted pool. The code below demonstrates how the data is unpacked to retrieve the exact moving average values:

/contracts/rules/UpdateRule.sol:78

78: /// @param _absoluteWeightGuardRail the maximum weight a token can have

79: function CalculateNewWeights(

80: int256[] calldata _prevWeights,

81: int256[] calldata _data,

82: address _pool,

83: int256[][] calldata _parameters,

84: uint64[] calldata _lambdaStore,

85: uint64 _epsilonMax,

86: uint64 _absoluteWeightGuardRail

87: ) external returns (int256[] memory updatedWeights) {

88: require(msg.sender == updateWeightRunner, "UNAUTH_CALC");

89:

90: QuantAMMUpdateRuleLocals memory locals;

91:

92: locals.numberOfAssets = _prevWeights.length;

93: locals.nMinusOne = locals.numberOfAssets - 1;

94: locals.lambda = new int128[](_lambdaStore.length);

95:

96: for (locals.i; locals.i < locals.lambda.length; ) {

97: locals.lambda[locals.i] = int128(uint128(_lambdaStore[locals.i]));

98: unchecked {

99: ++locals.i;

100: }

101: }

102:

103: locals.requiresPrevAverage = _requiresPrevMovingAverage() == REQ_PREV_MAVG_VAL;

104: locals.intermediateMovingAverageStateLength = locals.numberOfAssets;

105:

106: if (locals.requiresPrevAverage) {

107: unchecked {

108: locals.intermediateMovingAverageStateLength *= 2;

109: }

110: }

111:

112: locals.currMovingAverage = new int256[](locals.numberOfAssets);

113: locals.updatedMovingAverage = new int256[](locals.numberOfAssets);

114: @> locals.calculationMovingAverage = new int256[](locals.intermediateMovingAverageStateLength);

115:

116: @> locals.currMovingAverage = _quantAMMUnpack128Array(movingAverages[_pool], locals.numberOfAssets);

When the performUpdate function is called for the first time, it also stores the previous average if required. From the above code, at line 114, the new moving average will double in length if the previous average is needed. Then, at line 116, the current packed data is unpacked.

Assuming a two-asset weighted pool, the length of the packed moving average array becomes 2 due to the inclusion of previous average data, with the number of assets being 2. Let’s examine the unpacking code below.

contracts/QuantAMMStorage.sol:350

350: function _quantAMMUnpack128Array(

351: int256[] memory _sourceArray,

352: uint _targetArrayLength

353: ) internal pure returns (int256[] memory targetArray) {

354: // [0,1,2]

355: require(_sourceArray.length * 2 >= _targetArrayLength, "SRC!=TGT");

356: targetArray = new int256[](_targetArrayLength);

357: uint targetIndex;

358: uint sourceArrayLengthMinusOne = _sourceArray.length - 1;

359: bool divisibleByTwo = _targetArrayLength % 2 == 0;

360: for (uint i; i < _sourceArray.length; ) {

361: targetArray[targetIndex] = _sourceArray[i] >> 128;

362: unchecked {

363: ++targetIndex;

364: }

365: if ((!divisibleByTwo && i < sourceArrayLengthMinusOne) || divisibleByTwo) {

366: targetArray[targetIndex] = int256(int128(_sourceArray[i]));

367: }

368: unchecked {

369: ++i;

370: ++targetIndex;

371: }

372:

373: }

374:

375: if (!divisibleByTwo) {

376: targetArray[_targetArrayLength - 1] = int256(int128(_sourceArray[sourceArrayLengthMinusOne]));

378: }

379: }

380: }

This function is expected to return unpacked data, including both the current moving average and the previous moving average. Therefore, the final length of the array should be 4. In _targetArrayLength, the number of assets is passed, which is 2 in this case. However, this unpacking function will revert with [FAIL: panic: array out-of-bounds access (0x32)].

// SPDX-License-Identifier: GPL-3.0-or-later

pragma solidity ^0.8.24;

import "forge-std/Test.sol";

import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

import { IVault } from "@balancer-labs/v3-interfaces/contracts/vault/IVault.sol";

import { IVaultErrors } from "@balancer-labs/v3-interfaces/contracts/vault/IVaultErrors.sol";

import { PoolRoleAccounts } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol";

import { MockMinimumVarianceRule } from "../../contracts/mock/mockRules/MockMinimumVarianceRule.sol";

import { CastingHelpers } from "@balancer-labs/v3-solidity-utils/contracts/helpers/CastingHelpers.sol";

import { ArrayHelpers } from "@balancer-labs/v3-solidity-utils/contracts/test/ArrayHelpers.sol";

import { BalancerPoolToken } from "@balancer-labs/v3-vault/contracts/BalancerPoolToken.sol";

import { BaseVaultTest } from "@balancer-labs/v3-vault/test/foundry/utils/BaseVaultTest.sol";

import { QuantAMMWeightedPool } from "../../contracts/QuantAMMWeightedPool.sol";

import { QuantAMMWeightedPoolFactory } from "../../contracts/QuantAMMWeightedPoolFactory.sol";

import { QuantAMMWeightedPoolContractsDeployer } from "./utils/QuantAMMWeightedPoolContractsDeployer.sol";

import { PoolSwapParams, SwapKind } from "@balancer-labs/v3-interfaces/contracts/vault/VaultTypes.sol";

import { OracleWrapper } from "@balancer-labs/v3-interfaces/contracts/pool-quantamm/OracleWrapper.sol";

import { MockUpdateWeightRunner } from "../../contracts/mock/MockUpdateWeightRunner.sol";

import { MockMomentumRule } from "../../contracts/mock/mockRules/MockMomentumRule.sol";

import { MockAntiMomentumRule } from "../../contracts/mock/mockRules/MockAntiMomentumRule.sol";

import { MockChainlinkOracle } from "../../contracts/mock/MockChainlinkOracles.sol";

import "@balancer-labs/v3-interfaces/contracts/pool-quantamm/IQuantAMMWeightedPool.sol";

contract Pool3TokenTest is QuantAMMWeightedPoolContractsDeployer, BaseVaultTest {

using CastingHelpers for address[];

using ArrayHelpers for *;

uint64 public constant MAX_SWAP_FEE_PERCENTAGE = 10e16;

QuantAMMWeightedPoolFactory internal quantAMMWeightedPoolFactory;

function setUp() public override {

uint delay = 3600;

super.setUp();

(address ownerLocal, address addr1Local, address addr2Local) = (vm.addr(1), vm.addr(2), vm.addr(3));

owner = ownerLocal;

vm.startPrank(owner);

updateWeightRunner = new MockUpdateWeightRunner(owner, addr2, false);

chainlinkOracle = _deployOracle(1e18, delay);

chainlinkOracle2 = _deployOracle(1e18, delay);

updateWeightRunner.addOracle(OracleWrapper(chainlinkOracle));

updateWeightRunner.addOracle(OracleWrapper(chainlinkOracle2));

vm.stopPrank();

quantAMMWeightedPoolFactory = deployQuantAMMWeightedPoolFactory(

IVault(address(vault)),

365 days,

"Factory v1",

"Pool v1"

);

}

function test_previous_average_revert_case() public {

QuantAMMWeightedPoolFactory.NewPoolParams memory params = _createPoolParams();

params._initialWeights[0] = 0.03e18;

params._initialWeights[1] = 0.97e18;

(address quantAMMWeightedPool, ) = quantAMMWeightedPoolFactory.create(params);

vm.prank(owner);

updateWeightRunner.setApprovedActionsForPool(quantAMMWeightedPool,1);

chainlinkOracle.updateData(1e18, uint40(block.timestamp));

chainlinkOracle2.updateData(1e18, uint40(block.timestamp));

updateWeightRunner.performUpdate(quantAMMWeightedPool); // this one executed successfully

vm.expectRevert();

vm.warp(block.timestamp+1000);

updateWeightRunner.performUpdate(quantAMMWeightedPool); // this one will revert index out of bound

}

function _createPoolParams() internal returns (QuantAMMWeightedPoolFactory.NewPoolParams memory retParams) {

PoolRoleAccounts memory roleAccounts;

IERC20[] memory tokens = [address(dai), address(usdc)].toMemoryArray().asIERC20();

MockMinimumVarianceRule minimumvarianceRule = new MockMinimumVarianceRule(address(updateWeightRunner));

uint32[] memory weights = new uint32[]();

weights[0] = uint32(uint256(0.5e18));

weights[1] = uint32(uint256(0.4e18));

int256[] memory initialWeights = new int256[]();

initialWeights[0] = 0.5e18;

initialWeights[1] = 0.4e18;

uint256[] memory initialWeightsUint = new uint256[]();

initialWeightsUint[0] = 0.5e18;

initialWeightsUint[1] = 0.4e18;

uint64[] memory lambdas = new uint64[]();

lambdas[0] = 0.2e18;

int256[][] memory parameters = new int256[][]();

parameters[0] = new int256[]();

parameters[0][0] = 0.6e18;

address[][] memory oracles = new address[][]();

oracles[0] = new address[]();

oracles[1] = new address[]();

oracles[0][0] = address(chainlinkOracle);

oracles[1][0] = address(chainlinkOracle2);

retParams = QuantAMMWeightedPoolFactory.NewPoolParams(

"Pool With Donation",

"PwD",

vault.buildTokenConfig(tokens),

initialWeightsUint,

roleAccounts,

MAX_SWAP_FEE_PERCENTAGE,

address(0),

true,

false, // Do not disable unbalanced add/remove liquidity

ZERO_BYTES32,

initialWeights,

IQuantAMMWeightedPool.PoolSettings(

new IERC20[](2),

IUpdateRule(minimumvarianceRule),

oracles,

60,

lambdas,

0.2e18,

0.02e18, // absolute guard rail

0.2e18,

parameters,

address(0)

initialWeights,

3600,

new string[][]()

);

}

Run with command : forge test --mt test_previous_average_revert_case -vvv

Impact

In the case of minimum variance, where the previous average is also stored, the unpacking process will revert after the first call to the performUpdate function. This results in a DoS for all pools that use this UpdateRule.

Tools Used

Manual Review, Unit Testing

Recommendations

One of the Recommended Fix could be to handle this case as follows:

diff --git a/pkg/pool-quantamm/contracts/rules/UpdateRule.sol b/pkg/pool-quantamm/contracts/rules/UpdateRule.sol

index f495f8b..b9dbff2 100644

--- a/pkg/pool-quantamm/contracts/rules/UpdateRule.sol

+++ b/pkg/pool-quantamm/contracts/rules/UpdateRule.sol

@@ -7,7 +7,7 @@ import "./base/QuantammMathMovingAverage.sol";

import "../UpdateWeightRunner.sol";

import "./base/QuantammBasedRuleHelpers.sol";

import "@balancer-labs/v3-interfaces/contracts/pool-quantamm/IUpdateRule.sol";

@@ -31,6 +31,7 @@ abstract contract UpdateRule is QuantAMMMathGuard, QuantAMMMathMovingAverage, IU

string public name;

string[] public parameterDescriptions;

+ bool isFirstTime = true;

/// @dev struct to avoid stack too deep issues

/// @notice Struct to store local variables for the update rule

@@ -102,19 +103,24 @@ abstract contract UpdateRule is QuantAMMMathGuard, QuantAMMMathMovingAverage, IU

locals.requiresPrevAverage = _requiresPrevMovingAverage() == REQ_PREV_MAVG_VAL;

locals.intermediateMovingAverageStateLength = locals.numberOfAssets; // 2

+ locals.i =locals.numberOfAssets; // using locals.i for numberOfAssets

- if (locals.requiresPrevAverage) {

+ if (locals.requiresPrevAverage) {

unchecked {

locals.intermediateMovingAverageStateLength *= 2; // than we need to double the array size make it 10

}

+ // here need to check if this is not first call

+ if(!isFirstTime){

+ locals.i = locals.intermediateMovingAverageStateLength; // using local.i for number of assets

+ }

}

+ isFirstTime = false;

locals.currMovingAverage = new int256[](locals.numberOfAssets);

locals.updatedMovingAverage = new int256[](locals.numberOfAssets);

locals.calculationMovingAverage = new int256[](locals.intermediateMovingAverageStateLength);

- locals.currMovingAverage = _quantAMMUnpack128Array(movingAverages[_pool], locals.numberOfAssets)

+ locals.currMovingAverage = _quantAMMUnpack128Array(movingAverages[_pool], locals.i);

Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago

Submission Judgement Published

Validated

Assigned finding tags:

finding_CalculateNewWeights_DoS_when_requiresPrevAverage_is_true

Likelihood: Medium, all rules using previous average. Impact: High, DoS CalculateNewWeights

Prize pool breakdown

Total prize

49,600 OP

nSLOC

3,000

17 OP / LOC

High Medium

44,640

Low

4,960

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

View results

Support

FAQs

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

Give us feedback!