Summary
Weights are clamped wrongly so new weights are calculated wrongly as leading to broken weights.
https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-quantamm/contracts/rules/base/QuantammMathGuard.sol#L36
Vulnerability Details
QuantammMathGuard.sol#_guardQuantAMMWeights() function which guards QuantAMM weights is as follows.
function _guardQuantAMMWeights(
int256[] memory _weights,
int256[] calldata _prevWeights,
int256 _epsilonMax,
int256 _absoluteWeightGuardRail
) internal pure returns (int256[] memory guardedNewWeights) {
@>
@> _weights = _clampWeights(_weights, _absoluteWeightGuardRail);
guardedNewWeights = _normalizeWeightUpdates(_prevWeights, _weights, _epsilonMax);
}
As we can see above, _clampWeights() function clamps weights so that they go beyond the maximum/minimum.
This function is as follows.
/// @notice Applies guard rails (min value, max value) to weights and returns the normalized weights
/// @param _weights Raw weights
/// @return Clamped weights
function _clampWeights(
int256[] memory _weights,
int256 _absoluteWeightGuardRail
) internal pure returns (int256[] memory) {
unchecked {
uint weightLength = _weights.length;
if (weightLength == 1) {
return _weights;
}
int256 absoluteMin = _absoluteWeightGuardRail
@> int256 absoluteMax = ONE -
(PRBMathSD59x18.fromInt(int256(_weights.length - 1)).mul(_absoluteWeightGuardRail));
@> int256 sumRemainerWeight = ONE;
int256 sumOtherWeights
for (uint i; i < weightLength; ++i) {
if (_weights[i] < absoluteMin) {
@> _weights[i] = absoluteMin;
@> sumRemainerWeight -= absoluteMin;
} else if (_weights[i] > absoluteMax) {
@> _weights[i] = absoluteMax;
@> sumOtherWeights += absoluteMax;
}
}
@> if (sumOtherWeights != 0) {
@> int256 proportionalRemainder = sumRemainerWeight.div(sumOtherWeights);
for (uint i; i < weightLength; ++i) {
@> if (_weights[i] != absoluteMin) {
@> _weights[i] = _weights[i].mul(proportionalRemainder);
}
}
}
}
return _weights;
}
From doc, we can see that this function clamps to [minimum, maximum] and normalizes weights.
But above implementation is wrong.
Impact
This wrong implementation leads to not clamped weights and not normalized weights.
This makes the protocol to be broken.
Tools Used
Mannual review
Recommendations
We have to modify QuantammMathGuard.sol#_clampWeights() function as follows.
function _clampWeights(
int256[] memory _weights,
int256 _absoluteWeightGuardRail
) internal pure returns (int256[] memory) {
unchecked {
uint weightLength = _weights.length;
if (weightLength == 1) {
return _weights;
}
int256 absoluteMin = _absoluteWeightGuardRail;
int256 absoluteMax = ONE -
(PRBMathSD59x18.fromInt(int256(_weights.length - 1)).mul(_absoluteWeightGuardRail));
int256 aver;
int256 w_min = _weights[0], w_max = _weights[0];
for(uint i; i < weightLength; i++){
aver += _weights[i];
if(_weights[i] < w_min) w_min = _weights[i];
if(_weights[i] > w_max) w_max = _weights[i];
}
if(w_min >= absoluteMin && w_max <= absoluteMax){
return _weights;
}
aver /= weightLength;
int256 d_min = aver - w_min;
int256 d_max = w_max - aver;
int256 new_aver = int256(1e18).div(int256(weightLength));
int256 n_min = new_aver - absoluteMin;
int256 n_max = absoluteMax - new_aver;
int256 rate;
if(d_min != 0){
rate = n_min.div(d_min);
}else{
rate = type(int256).max;
}
if(d_max != 0){
int256 t_rate = n_max.div(d_max);
if(t_rate < rate){
rate = t_rate;
}
}
for(uint i; i < weightLength; i++){
_weights[i] = (_weights[i] - aver).mul(rate) + new_aver;
}
}
}
Description of recommendation
We convert weights to w'[i] = (w[i] - a) * lamda + 1 / n.
Here, a = sum{w} / n. lamda = min{(1 / n - absoluteMin) / (a - w_min), (absoluteMax - 1 / n) / (w_max - a)}.
w_min = min{w}. w_max = max{w}.
Then, sum{w'} = 1. min{w'} >= absoluteMin and max{w'} <= absoluteMax.
