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

Underflow leading to wrong calculation

rzizah

Underflow in computeBalance, _getNormalizedWeights functions due to missing input sanitation

Summary

In computeBalance and _getNormalizedWeights weights depend on the

  • previous weights

  • multiplier

  • timeSinceLastUpdate

Calculating timeSinceLastUpdate checks if an pdate is needed and set multiplierTime to lastPossibleInterpolationTime if block.timestamp is bigger than it
then subtract lastUpdateIntervalTime from it which in normal cases <= lastPossibleInterpolationTime depends on whether the protocol has been just initialized or an update is needed but there is an edge case where there is no check for that value not exceeding lastPossibleInterpolationTime

knowing how this value is applied; it's set through

  • _setInitialWeights

    • in the case of initial weights both lastPossibleInterpolationTime and lastUpdateIntervalTime is the same block.timestamp

  • setWeights

    • called through _calculateMultiplerAndSetWeights and setWeightManually

      • _calculateMultiplerAndSetWeights has gaurd as value is assured to be >= block.timestamp

      • In setWeightManually value goes directly to setWeights function without any sanitisation.

Attack Scenario

Assume there was a long pause and weights have to be set again manually as it depends on previous recent weights and multiplier so the solution is to set it manually

as done with setting initial weights during initialization manager tries to add the correct weights with the initial time (both lastInetpolationTimePossible and lastUpdateIntervalTime is the same until the update)
however issue arises as

  • lastPossibleInterpolationTime is inputted and not calculated as block.timestamp so

  • manager checks the current timestamp for example 1700000000 add an extra 5 seconds to be safe as the normal execution time is 2 seconds.

  • due to sequencer downtime or congestion could push the block.timestamp at the execution to 1700000010 or beyond.
    so values recorded after settingWeights are

  • lastPossibleInterpolationTime = 1700000005

  • lastUpdateIntervalTime = 1700000010
    in functions specified above have unchecked blocks calculating the timeSinceLastUpdate so underflow happens and a huge timeSinceLastUpdate is recorded returning very large weights

This applies to onSwap function but as this function doesn't have an unchecked block for the calculation it revert

in the scenario, it's assumed that all values inputted by the manager are correct and not malicious and the error arises because of the wrong implementation (missing guards).

POC

add this in any test contract ex: QuantAMMWeightedPool2TokenTest
it has all needed helper functions with the change of the value of block.timestamp with mocked values

​
int256 internal _normalizedFirstFourWeights ;
//packed: [weight5,weight6,weight7,weight8,multiplier5,multiplier6,multiplier7,multiplier8]
int256 internal _normalizedSecondFourWeights ;
​
​
// test function
function testUnderflow() public{
//packing first 4 weights
int256[] memory first4weights = new int256[]();
first4weights[0] = 0.2e18;
first4weights[1] = 0.1e18;
first4weights[2] = 0.1e18;
first4weights[3] = 0.1e18;
first4weights[4] = 0.1e18;
first4weights[5] = 0.2e18;
first4weights[6] = 0.1e18;
first4weights[7] = 0.1e18;
_normalizedFirstFourWeights = quantAMMPack32Array(first4weights)[0];
​
//packing the 2nd 4 weights
int256[] memory second4weights = new int256[]();
second4weights[0] = 0.1e18;
second4weights[1] = 0.2e18;
second4weights[2] = 0.1e18;
second4weights[3] = 0.1e18;
second4weights[4] = 0.1e18;
second4weights[5] = 0.2e18;
second4weights[6] = 0.1e18;
second4weights[7] = 0.1e18;
_normalizedSecondFourWeights = quantAMMPack32Array(second4weights)[0];
​
//checking the weights with output
uint256[] memory weights = new uint256[]();
weights[0] = 0.2e18;
weights[1] = 0.1e18;
weights[2] = 0.1e18;
weights[3] = 0.1e18;
weights[4] = 0.1e18;
weights[5] = 0.2e18;
weights[6] = 0.1e18;
weights[7] = 0.1e18;
assertNotEq(_getNormalizedWeights(),weights);
}
​
​
uint40 mockedtimestamp = 1700000010;
function _getNormalizedWeights() public view virtual returns (uint256[] memory) {
uint256 totalTokens = 4;
uint256[] memory normalizedWeights = new uint256[]();
​
uint40 multiplierTime = mockedtimestamp;
uint40 lastInterpolationTime = (mockedtimestamp - 5) ;// here its assume the delay of 10 seconds considering the safe gaurd of the manager of 5 seconds added
​
if (multiplierTime >= lastInterpolationTime) {
//we have gone beyond the first variable hitting the guard rail. We cannot interpolate any further and an update is needed
multiplierTime = lastInterpolationTime;
}
​
unchecked {
uint256 timeSinceLastUpdate = uint256(
multiplierTime - mockedtimestamp
);
​
int256[] memory firstFourWeights = quantAMMUnpack32(_normalizedFirstFourWeights);
uint256 tokenIndex = totalTokens;
if (totalTokens > 4) {
tokenIndex = 4;
}
​
//not using _calculateCurrentBlockWeight as hardcoded you avoid 1 calc gas
normalizedWeights[0] = calculateBlockNormalisedWeight(
firstFourWeights[0],
firstFourWeights[tokenIndex],
timeSinceLastUpdate
);
normalizedWeights[1] = calculateBlockNormalisedWeight(
firstFourWeights[1],
firstFourWeights[tokenIndex + 1],
timeSinceLastUpdate
);
​
if (totalTokens > 2) {
normalizedWeights[2] = calculateBlockNormalisedWeight(
firstFourWeights[2],
firstFourWeights[tokenIndex + 2],
timeSinceLastUpdate
);
} else {
return normalizedWeights;
}
if (totalTokens > 3) {
normalizedWeights[3] = calculateBlockNormalisedWeight(
firstFourWeights[3],
firstFourWeights[tokenIndex + 3],
timeSinceLastUpdate
);
} else {
return normalizedWeights;
}
​
//avoid unneccessary SLOAD
if (totalTokens == 4) {
return normalizedWeights;
}
​
int256[] memory secondFourWeights = quantAMMUnpack32(_normalizedSecondFourWeights);
uint256 moreThan4Tokens = totalTokens - 4;
​
if (totalTokens > 4) {
normalizedWeights[4] = calculateBlockNormalisedWeight(
secondFourWeights[0],
secondFourWeights[moreThan4Tokens],
timeSinceLastUpdate
);
} else {
return normalizedWeights;
}
if (totalTokens > 5) {
normalizedWeights[5] = calculateBlockNormalisedWeight(
secondFourWeights[1],
secondFourWeights[moreThan4Tokens + 1],
timeSinceLastUpdate
);
} else {
return normalizedWeights;
}
if (totalTokens > 6) {
normalizedWeights[6] = calculateBlockNormalisedWeight(
secondFourWeights[2],
secondFourWeights[moreThan4Tokens + 2],
timeSinceLastUpdate
);
} else {
return normalizedWeights;
}
if (totalTokens > 7) {
normalizedWeights[7] = calculateBlockNormalisedWeight(
secondFourWeights[3],
secondFourWeights[moreThan4Tokens + 3],
timeSinceLastUpdate
);
} else {
return normalizedWeights;
}
​
return normalizedWeights;
}
}
​
​
uint256 constant ONE = 1e18;
// helper functions
function mulUp(uint256 a, uint256 b) public pure returns (uint256 result) {
// Multiplication overflow protection is provided by Solidity 0.8.x.
uint256 product = a * b;
​
// Equivalent to:
// result = product == 0 ? 0 : ((product - 1) / FixedPoint.ONE) + 1
assembly ("memory-safe") {
result := mul(iszero(iszero(product)), add(div(sub(product, 1), ONE), 1))
}
}
​
function mulDown(uint256 a, uint256 b) public view returns (uint256) {
// Multiplication overflow protection is provided by Solidity 0.8.x.
uint256 product = a * b;
​
return product / ONE;
}
function calculateBlockNormalisedWeight(
int256 weight,
int256 multiplier,
uint256 timeSinceLastUpdate
) public view returns (uint256) {
//multiplier is always below 1 which is int128, we multiply by 1e18 for rounding as muldown / 1e18 at the end.
int256 multiplierScaled18 = multiplier * 1e18;
if (multiplier > 0) {
return uint256(weight) + mulDown(uint256(multiplierScaled18), timeSinceLastUpdate);
} else {
//CYFRIN H02
return uint256(weight) - mulUp(uint256(-multiplierScaled18), timeSinceLastUpdate);
}
}
​
//packing functions
/// @notice Packs an array of 32 bit integers into an array of 256 bit integers
/// @param _sourceArray the array to pack
function quantAMMPack32Array(int256[] memory _sourceArray) internal pure returns (int256[] memory targetArray) {
uint targetArrayLength;
uint storageIndex;
uint nonStickySourceLength;
​
//logic if more than 1 slot is required to store the array
if (_sourceArray.length >= 8) {
for (uint i = _sourceArray.length; i >= 8; ) {
unchecked {
if (i % 8 == 0) {
nonStickySourceLength = i;
break;
}
--i;
}
}
​
//add one for the sticky end to be dealt with later
if (_sourceArray.length != nonStickySourceLength) {
unchecked {
targetArrayLength = (nonStickySourceLength / 8) + 1;
}
} else {
unchecked {
targetArrayLength = (nonStickySourceLength) / 8;
}
}
​
targetArray = new int256[](targetArrayLength);
​
for (uint i; i < nonStickySourceLength; ) {
unchecked {
targetArray[storageIndex] = quantAMMPackEight32(
int256(_sourceArray[i] / 1e9),
int256(_sourceArray[i + 1] / 1e9),
int256(_sourceArray[i + 2] / 1e9),
int256(_sourceArray[i + 3] / 1e9),
int256(_sourceArray[i + 4] / 1e9),
int256(_sourceArray[i + 5] / 1e9),
int256(_sourceArray[i + 6] / 1e9),
int256(_sourceArray[i + 7] / 1e9)
);
​
i += 8;
++storageIndex;
}
}
}
​
if (targetArrayLength == 0) {
unchecked {
if (_sourceArray.length <= 8) {
targetArrayLength = 1;
} else {
targetArrayLength = (nonStickySourceLength / 8) + 1;
}
​
targetArray = new int256[](targetArrayLength);
}
}
//pack up to 7 sticky ends
uint stickyEndElems = _sourceArray.length - nonStickySourceLength;
if (stickyEndElems > 0) {
uint offset = 224;
int256 packed;
for (uint i = nonStickySourceLength; i < _sourceArray.length; ) {
unchecked {
int256 elem = _sourceArray[i] / 1e9;
require(elem <= MAX32 && elem >= MIN32, "Overflow");
packed |= int256(uint256(elem << 224) >> 224) << offset;
offset -= 32;
++i;
}
}
targetArray[storageIndex] = packed;
}
}
​
// max32 and min32
int256 private constant MAX32 = int256(type(int32).max);
int256 private constant MIN32 = int256(type(int32).min);
function quantAMMPackEight32(
int256 _firstInt,
int256 _secondInt,
int256 _thirdInt,
int256 _fourthInt,
int256 _fifthInt,
int256 _sixthInt,
int256 _seventhInt,
int256 _eighthInt
) internal pure returns (int256 packed) {
require(
_firstInt <= MAX32 &&
_firstInt >= MIN32 &&
_secondInt <= MAX32 &&
_secondInt >= MIN32 &&
_thirdInt <= MAX32 &&
_thirdInt >= MIN32 &&
_fourthInt <= MAX32 &&
_fourthInt >= MIN32 &&
_fifthInt <= MAX32 &&
_fifthInt >= MIN32 &&
_sixthInt <= MAX32 &&
_sixthInt >= MIN32 &&
_seventhInt <= MAX32 &&
_seventhInt >= MIN32 &&
_eighthInt <= MAX32 &&
_eighthInt >= MIN32,
"Overflow"
);
​
int256 firstPacked = int256(uint256(_firstInt << 224) >> 224) << 224;
int256 secondPacked = int256(uint256(_secondInt << 224) >> 224) << 192;
int256 thirdPacked = int256(uint256(_thirdInt << 224) >> 224) << 160;
int256 fourthPacked = int256(uint256(_fourthInt << 224) >> 224) << 128;
int256 fifthPacked = int256(uint256(_fifthInt << 224) >> 224) << 96;
int256 sixthPacked = int256(uint256(_sixthInt << 224) >> 224) << 64;
int256 seventhPacked = int256(uint256(_seventhInt << 224) >> 224) << 32;
int256 eighthPacked = int256(uint256(_eighthInt << 224) >> 224);
​
packed =
firstPacked |
secondPacked |
thirdPacked |
fourthPacked |
fifthPacked |
sixthPacked |
seventhPacked |
eighthPacked;
}
function quantAMMUnpack32(int256 sourceElem) internal pure returns (int256[] memory targetArray) {
targetArray = new int256[](8);
targetArray[0] = (sourceElem >> 224) * 1e9;
targetArray[1] = int256(int32(sourceElem >> 192)) * 1e9;
targetArray[2] = int256(int32(sourceElem >> 160)) * 1e9;
targetArray[3] = int256(int32(sourceElem >> 128)) * 1e9;
targetArray[4] = int256(int32(sourceElem >> 96)) * 1e9;
targetArray[5] = int256(int32(sourceElem >> 64)) * 1e9;
targetArray[6] = int256(int32(sourceElem >> 32)) * 1e9;
targetArray[7] = int256(int32(sourceElem)) * 1e9;
​
return targetArray;
}
function quantAMMUnpack32Array(
int256[] memory _sourceArray,
uint _targetArrayLength
) internal pure returns (int256[] memory targetArray) {
require(_sourceArray.length * 8 >= _targetArrayLength, "SRC!=TGT");
targetArray = new int256[](_targetArrayLength);
uint targetIndex;
uint sourceArrayLengthMinusOne = _sourceArray.length - 1;
bool divisibleByEight = _targetArrayLength % 8 == 0;
uint stickyEndSourceElem;
​
//more than the first slot so need to loop
if (_targetArrayLength > 8) {
for (uint i; i < _sourceArray.length; ) {
if (divisibleByEight || i < sourceArrayLengthMinusOne) {
unchecked {
int256 sourceElem = _sourceArray[i];
targetArray[targetIndex] = (sourceElem >> 224) * 1e9;
++targetIndex;
​
targetArray[targetIndex] = int256(int32(sourceElem >> 192)) * 1e9;
++targetIndex;
​
targetArray[targetIndex] = int256(int32(sourceElem >> 160)) * 1e9;
++targetIndex;
​
targetArray[targetIndex] = int256(int32(sourceElem >> 128)) * 1e9;
++targetIndex;
​
targetArray[targetIndex] = int256(int32(sourceElem >> 96)) * 1e9;
++targetIndex;
​
targetArray[targetIndex] = int256(int32(sourceElem >> 64)) * 1e9;
++targetIndex;
​
targetArray[targetIndex] = int256(int32(sourceElem >> 32)) * 1e9;
++targetIndex;
​
targetArray[targetIndex] = int256(int32(sourceElem)) * 1e9;
++targetIndex;
}
}
unchecked {
++i;
}
}
//get sticky end slot
if (!divisibleByEight) {
unchecked {
stickyEndSourceElem = _sourceArray.length - 1;
}
}
} else if (_targetArrayLength == 8) {
//effiency at the price of increased function length works out cheaper
//hardcoded index access is cheaper than a loop
int256 sourceElem = _sourceArray[0];
targetArray[0] = (sourceElem >> 224) * 1e9;
targetArray[1] = int256(int32(sourceElem >> 192)) * 1e9;
targetArray[2] = int256(int32(sourceElem >> 160)) * 1e9;
targetArray[3] = int256(int32(sourceElem >> 128)) * 1e9;
targetArray[4] = int256(int32(sourceElem >> 96)) * 1e9;
targetArray[5] = int256(int32(sourceElem >> 64)) * 1e9;
targetArray[6] = int256(int32(sourceElem >> 32)) * 1e9;
targetArray[7] = int256(int32(sourceElem)) * 1e9;
}
​
// deal with up to 7 sticky end elements
if (!divisibleByEight) {
unchecked {
uint offset = 224;
for (uint i = targetIndex; i < targetArray.length; ) {
targetArray[i] = int256(int32(_sourceArray[stickyEndSourceElem] >> offset)) * 1e9;
offset -= 32;
++i;
}
}
}

you'll notice that weights are broken giving wrong values far away from real ones breaking the core logic of the protocol.

Impact

  • broken weights calculations.

  • total breakage of the protocol as its core is broken.

Tools Used

manual review

Recommendations

in setWeights function add this check

++ require(_lastInterpolationTimePossible >= block.timestamp, " WronglasstInterpolationTimePossible value");
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas / Admin is trusted / Pool creation is trusted / User mistake / Suppositions

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelyhood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Appeal created

rzizah Submitter
10 months ago
rzizah Submitter
10 months ago
rzizah Submitter
10 months ago
n0kto Lead Judge
10 months ago
n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas / Admin is trusted / Pool creation is trusted / User mistake / Suppositions

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelyhood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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

Give us feedback!