QuantAMM

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

packing and unpacking mechanism is broken

Summary

The packing function takes the array and packs it, and the unpacking function takes the packed array and inputted length of the original array and checks if it's valid with this require

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

but this doesn't ensure it is valid

assume there is a packed array with original length of 8 after the packing length is 4
going to unpacking function with array length = 4 and targetArrayLength = 7

packed array [0] [1] [2] [3]
The value packed 0,1 2,3 4,5 6,7

require successfully pass as 8 >= 7 and function execute ignoring the value stored at (lastindex-1)

packed array [0] [1] [2] [3]
The value retrieved 0,1 2,3 4,5 7

the same applies with odd length array assuming original array length of 7 then after packing = 4

packed array [0] [1] [2] [3]
The value packed 0,1 2,3 4,5 6

going to the unpacking function with targetArrayLength = 8 an extra empty value will be in (lastindex -1)

packed array [0] [1] [2] [3]
The value retrieved 0,1 2,3 4,5 (0),7

POC

Add this test in QuantAMMStorageTest contract

function testArrayWrongLength() public view {
// Define the target values
int256[] memory targetValues = new int256[]();
targetValues[0] = 1e18;
targetValues[1] = 2e18;
targetValues[2] = 3e18;
targetValues[3] = 4e18;
targetValues[4] = 5e18;
targetValues[5] = 6e18;
targetValues[6] = 7e18;
// Call the ExternalEncodeDecode128Array function from the contract
int256[] memory redecoded = mockQuantAMMStorage.ExternalEncodeDecode128Array(targetValues, 8);
assertEq(targetValues[0], redecoded[0]);
assertEq(targetValues[1], redecoded[1]);
assertEq(targetValues[2], redecoded[2]);
assertEq(targetValues[3], redecoded[3]);
assertEq(targetValues[4], redecoded[4]);
assertEq(targetValues[5], redecoded[5]);
assertEq(0 , redecoded[6]);
assertEq(targetValues[6], redecoded[7]);
//testing with original of 8 and retrieved 7
int256[] memory targetValues1 = new int256[]();
targetValues1[0] = 1e18;
targetValues1[1] = 2e18;
targetValues1[2] = 3e18;
targetValues1[3] = 4e18;
targetValues1[4] = 5e18;
targetValues1[5] = 6e18;
targetValues1[6] = 7e18;
targetValues1[7] = 8e18;
// Call the ExternalEncodeDecode128Array function from the contract
int256[] memory redecoded1 = mockQuantAMMStorage.ExternalEncodeDecode128Array(targetValues1, 7);
assertEq(targetValues1[0], redecoded1[0]);
assertEq(targetValues1[1], redecoded1[1]);
assertEq(targetValues1[2], redecoded1[2]);
assertEq(targetValues1[3], redecoded1[3]);
assertEq(targetValues1[4], redecoded1[4]);
assertEq(targetValues1[5], redecoded1[5]);
// asset 6 deleted
assertEq(targetValues1[7], redecoded1[6]);
}

Impact

  • broken function leads to wrong values packed and retrieved

Tools Used

manual review

Recommendations

one solution is storing the length of the original array while packing at the last index
ex: original array length of 8 then after packing the length is 5 with the first 4 being the packed values and 5th index for the length and in unpacking function just read the last index this way it would be impossible for such collisions to happen in packing and unpacking

in _quantAMMPack128Array function at the end

++ targetArray[storageIndex] = _sourceArray.length;
// incase of sticky end
++ targetArray[storageIndex + 1] = _sourceArray.length;

in _quantAMMUnpack128Array instead of requiring

++ _targetArrayLength = _sourceArray[_sourceArray.length -1];
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.

Support

FAQs

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

Give us feedback!