Summary
The validParameters function currently performs basic validation, such as ensuring that parameters are positive and have the correct array lengths, but it does not enforce upper bounds on the values of the parameters. This omission can lead to situations where the parameters are too large, causing intermediate calculations to overflow.
Vulnerability Details
The ChannelFollowingUpdateRule contract is using PRBMathSD59x18 for int256 arithmetic:
using PRBMathSD59x18 for int256;
That means the bound for int256 is between MIN_SD59x18 and MAX_SD59x18.
int256 internal MAX_SD59x18 =
57896044618658097711785492504343953926634992332820282019728_792003956564819967;
int256 internal MIN_SD59x18 =
-57896044618658097711785492504343953926634992332820282019728_792003956564819968;
The validParameters function currently only performs basic validation:
for (uint i = 0; i < baseLength; i++) {
if (_parameters[0][i] <= 0) return false;
if (_parameters[1][i] <= 0) return false;
if (_parameters[2][i] <= 0) return false;
if (_parameters[3][i] <= 0) return false;
if (_parameters[4][i] <= 0) return false;
if (_parameters[5][i] <= 0) return false;
}
That means all parameters' bounds are between 1 and MAX_SD59x18. Add following test in ChannelFollowingUpdateRuleTest and it will prove this point:
function testParamsBounds(
int256 param1,
int256 param2,
int256 param3,
int256 param4,
int256 param5,
int256 param6
)public{
int256[][] memory parameters = new int256[][]();
for (uint i = 0; i < 6; i++) {
parameters[i] = new int256[]();
parameters[i][0] = PRBMathSD59x18.fromInt(1);
}
parameters[6] = new int256[]();
parameters[6][0] = PRBMathSD59x18.fromInt(0);
parameters[0][0] = PRBMathSD59x18.fromInt(bound(param1, 1, maxScaledFixedPoint18()));
parameters[1][0] = PRBMathSD59x18.fromInt(bound(param2, 1, maxScaledFixedPoint18()));
parameters[2][0] = PRBMathSD59x18.fromInt(bound(param3, 1, maxScaledFixedPoint18()));
parameters[3][0] = PRBMathSD59x18.fromInt(bound(param4, 1, maxScaledFixedPoint18()));
parameters[4][0] = PRBMathSD59x18.fromInt(bound(param5, 1, maxScaledFixedPoint18()));
parameters[5][0] = PRBMathSD59x18.fromInt(bound(param6, 1, maxScaledFixedPoint18()));
bool valid = rule.validParameters(parameters);
assertTrue(valid);
}
Run forge test -vvv --match-test testParamsBounds , the test passes:
Ran 1 test for test/foundry/rules/QuantAMMChannelFollowing.t.sol:ChannelFollowingUpdateRuleTest
[PASS] testParamsBounds(int256,int256,int256,int256,int256,int256) (runs: 10002, μ: 43923, ~: 44257)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 779.77ms (776.28ms CPU time)
However, it may lead to overflow in _getWeights function.
The _getWeights function performs complex mathematical operations, including:
Exponentiation (exp, _pow),
Multiplication and division,
Summation of weighted signals.
If the parameters are not properly bounded, these operations can easily produce intermediate results that exceed the range of int256 (which is approximately [-2^255, 2^255 - 1]). For example:
A large kappa value could amplify the signal, causing the weight update to overflow.
A small width value could make the envelope calculation (exp(-gradientSquared / (2 * width^2))) produce extremely large results, leading to overflow in subsequent multiplications.
Add following test case in ChannelFollowingUpdateRuleTest:
function testCalculateWeightsWithFuzzedValue(
int256 param1,
int256 param2,
int256 param3,
int256 param4,
int256 param5,
int256 param6
) public {
int256[][] memory parameters = new int256[][]();
for (uint i = 0; i < 6; i++) {
parameters[i] = new int256[]();
parameters[i][0] = PRBMathSD59x18.fromInt(1);
}
parameters[6] = new int256[]();
parameters[6][0] = PRBMathSD59x18.fromInt(0);
parameters[0][0] = PRBMathSD59x18.fromInt(bound(param1, 1, maxScaledFixedPoint18()));
parameters[1][0] = PRBMathSD59x18.fromInt(bound(param2, 1, maxScaledFixedPoint18()));
parameters[2][0] = PRBMathSD59x18.fromInt(bound(param3, 1, maxScaledFixedPoint18()));
parameters[3][0] = PRBMathSD59x18.fromInt(bound(param4, 1, maxScaledFixedPoint18()));
parameters[4][0] = PRBMathSD59x18.fromInt(bound(param5, 1, maxScaledFixedPoint18()));
parameters[5][0] = PRBMathSD59x18.fromInt(bound(param6, 1, maxScaledFixedPoint18()));
bool valid = rule.validParameters(parameters);
assertTrue(valid);
int256[] memory prevAlphas = new int256[]();
prevAlphas[0] = PRBMathSD59x18.fromInt(1);
prevAlphas[1] = PRBMathSD59x18.fromInt(2);
int256[] memory prevMovingAverages = new int256[]();
prevMovingAverages[0] = PRBMathSD59x18.fromInt(3);
prevMovingAverages[1] = PRBMathSD59x18.fromInt(4);
int256[] memory movingAverages = new int256[]();
movingAverages[0] = PRBMathSD59x18.fromInt(3);
movingAverages[1] = PRBMathSD59x18.fromInt(4);
int128[] memory lambdas = new int128[]();
lambdas[0] = int128(0.9e18);
int256[] memory prevWeights = new int256[]();
prevWeights[0] = 0.5e18;
prevWeights[1] = 0.5e18;
int256[] memory data = new int256[]();
data[0] = PRBMathSD59x18.fromInt(5);
data[1] = PRBMathSD59x18.fromInt(6);
mockPool.setNumberOfAssets(2);
rule.initialisePoolRuleIntermediateValues(
address(mockPool),
prevMovingAverages,
prevAlphas,
mockPool.numAssets()
);
rule.CalculateUnguardedWeights(prevWeights, data, address(mockPool), parameters, lambdas, movingAverages);
int256[] memory resultWeights = rule.GetResultWeights();
}
All parameters are fuzzed values which are between 1 and MAX_SD59x18 . Run forge test -vvv --match-test testCalculateWeightsWithFuzzedValue , the result is:
Ran 1 test for test/foundry/rules/QuantAMMChannelFollowing.t.sol:ChannelFollowingUpdateRuleTest
[FAIL: PRBMath__MulDivFixedPointOverflow(28948022309329048855696955016157805541946181829897863632468348167255069033018 [2.894e76]); counterexample: calldata=0x009fae92000000000000000000000000000000000000000000000000000000300d00faa9ffffffffffffffffffffffffffffffff6cb3fac52d23455f3157dad0c79b573200000000000000000000000000000000000004e6976a9dc33059fd28a89f57950000000000000000000000000000000000593a0fbe961337b38ccfff39e45dda7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff000000000000000000000000000000000000000030796375954f4cf66038bf96 args=[206376598185 [2.063e11], -195791236014171421371645397194854672590 [-1.957e38], 99398976920519518597419552954261 [9.939e31], 463292047572630661258667552856628698 [4.632e35], 57896044618658097711785492504343953926634992332820282019728792003956564819967 [5.789e76], 15002030178907696803694034838 [1.5e28]]] testCalculateWeightsWithFuzzedValue(int256,int256,int256,int256,int256,int256) (runs: 0, μ: 0, ~: 0)
PRBMath__MulDivFixedPointOverflow error happens in _getWeights function.
Impact
The Impact is HIGH, the likelihood is LOW. So the Severity is MEDIUM.
Tools Used
Manual Review
Recommendations
Consider adding Upper Bounds Checking in validParameters.
