Summary
The DifferenceMomentumUpdateRule contract allows multiple initializations and fails to maintain consistent state between initializations, leading to corrupted pool states and incorrect weight calculations.
Vulnerability Details
function _setInitialIntermediateValues(
address _poolAddress,
int256[] memory _initialValues,
uint _numberOfAssets
) internal override {
require(_initialValues.length == _numberOfAssets, "Invalid initial values");
uint movingAverageLength = shortMovingAverages[_poolAddress].length;
if (movingAverageLength == 0 || _initialValues.length == _numberOfAssets) {
shortMovingAverages[_poolAddress] = _quantAMMPack128Array(_initialValues);
} else {
revert("Invalid set moving avg");
}
}
In the _setInitialIntermediateValues function in DifferenceMomentumUpdateRule there is no state tracking for initialization, the condition movingAverageLength == 0 || _initialValues.length == _numberOfAssets allows multiple initializations, and there is no validation of values after packing.
function initialisePoolRuleIntermediateValues(
address _poolAddress,
int256[] memory _newMovingAverages,
int256[] memory _newInitialValues,
uint _numberOfAssets
) external {
require(msg.sender == _poolAddress || msg.sender == updateWeightRunner, "UNAUTH");
_setInitialMovingAverages(_poolAddress, _newMovingAverages, _numberOfAssets);
_setInitialIntermediateValues(_poolAddress, _newInitialValues, _numberOfAssets);
}
In the initialisePoolRuleIntermediateValues function in UpdateRule there is no locking mechanism after the first initialization that allows multiple calls from authorized addresses and there is no state validation after initialization.
POC
Add this to QuantAMMDifferenceMomentum.t.sol and run it forge test --match-test "testInconsistentStateAfterInitialization|testStateCorruptionViaMultipleInitializations" -vvvv.
function testInconsistentStateAfterInitialization() public {
address poolAddress = address(mockPool);
int256[] memory initialValues = new int256[]();
initialValues[0] = 1e18;
initialValues[1] = 1e18;
vm.startPrank(owner);
rule.initialisePoolRuleIntermediateValues(poolAddress, initialValues, initialValues, 2);
int256[] memory storedValues = rule.GetResultWeights();
assertEq(storedValues.length, 0);
assertNotEq(storedValues.length, initialValues.length);
int256[][] memory parameters = new int256[][]();
parameters[0] = new int256[]();
parameters[0][0] = PRBMathSD59x18.fromInt(1);
parameters[1] = new int256[]();
parameters[1][0] = 0.5e18;
rule.CalculateUnguardedWeights(
initialValues,
initialValues,
poolAddress,
parameters,
new int128[](1),
initialValues
);
vm.stopPrank();
}
function testStateCorruptionViaMultipleInitializations() public {
address poolAddress = address(mockPool);
int256[] memory initialValues = new int256[]();
initialValues[0] = 1e18;
initialValues[1] = 1e18;
vm.startPrank(owner);
rule.initialisePoolRuleIntermediateValues(poolAddress, initialValues, initialValues, 2);
rule.initialisePoolRuleIntermediateValues(poolAddress, initialValues, initialValues, 2);
int256[] memory storedValues = rule.GetResultWeights();
assertEq(storedValues.length, 0);
int256[][] memory parameters = new int256[][]();
parameters[0] = new int256[]();
parameters[0][0] = PRBMathSD59x18.fromInt(1);
parameters[1] = new int256[]();
parameters[1][0] = 0.5e18;
rule.CalculateUnguardedWeights(
initialValues,
initialValues,
poolAddress,
parameters,
new int128[](1),
initialValues
);
vm.stopPrank();
}
Trace:
├─ [2773] MockDifferenceMomentumRule::GetResultWeights() [staticcall]
│ └─ ← [Return] []
An empty array is returned ([]) when it should contain weights, this proves the state was not saved correctly after initialization.
├─ [93103] MockDifferenceMomentumRule::initialisePoolRuleIntermediateValues(...)
│ └─ ← [Return]
├─ [5357] MockDifferenceMomentumRule::initialisePoolRuleIntermediateValues(...)
│ └─ ← [Return]
Two calls to initialisePoolRuleIntermediateValues were executed successfully, no revert on the second initialization, different gas used (93103 vs 5357) indicating state changed.
├─ [81276] MockDifferenceMomentumRule::CalculateUnguardedWeights(...)
│ └─ ← [Return]
The contract still accepts the calculation even though the state is invalid and there is no state validation before the calculation.
This allows manipulation of the state pool which can lead to invalid weight calculations.
Impact
Pool state corruption leading to incorrect weight calculations
Tools Used
Recommendations
Implement strict initialization controls.
contract DifferenceMomentumUpdateRule {
mapping(address => bool) private initialized;
function _setInitialIntermediateValues(
address _poolAddress,
int256[] memory _initialValues,
uint _numberOfAssets
) internal override {
require(!initialized[_poolAddress], "Already initialized");
require(_initialValues.length == _numberOfAssets, "Invalid initial values");
shortMovingAverages[_poolAddress] = _quantAMMPack128Array(_initialValues);
int256[] memory unpacked = _quantAMMUnpack128Array(
shortMovingAverages[_poolAddress],
_numberOfAssets
);
require(unpacked.length == _numberOfAssets, "Invalid packed state");
initialized[_poolAddress] = true;
emit PoolInitialized(_poolAddress, _initialValues);
}
modifier onlyUninitialized(address pool) {
require(!initialized[pool], "Already initialized");
_;
}
}
