Submission Details
Severity:
QA: I demonstrate one method how to check the logic inside the `for` loop works correctly & returns the expected results.
QA: I demonstrate one method how to check the logic inside the
forloop works correctly & returns the expected results.
Library function under scrutiny:
src/libraries/LibLastReserveBytes::storeLastReserves()
Modified test function used to scrutinize the library function:
test/libraries/LibLastReserveBytes.t.sol::testEmaFuzz_storeAndRead()
I initially thought the logic contained a bug but after brainstorming I added some modifications to both the test function and the library function in question and the results were crystal clear, it's NOT a bug, it works perfectly fine.
But this report could potentially serve as a valuable learning opportunity for beginner web3 security auditoors to help them with some of their future PoC tests.
I was convinced this was a bug, didn't understand the test results when I tried to prove my suspicions, then I started adding stuff to provide me with more info during test runs, which quickly helped me realise nope it's definitely NOT a bug, the dev is a wizard. (also a yul/assembly wizard it seems).
Cool.
The library function in question:
function storeLastReserves(bytes32 slot, uint40 lastTimestamp, uint256[] memory lastReserves) internal returns(uint256 counter_even, uint256 counter_odd) {
// Potential optimization – shift reserve bytes left to perserve extra decimal precision.
uint8 n = uint8(lastReserves.length);
bytes16[] memory reserves = new bytes16[](n);
for (uint256 i; i < n; ++i) {
reserves[i] = lastReserves[i].fromUInt();
}
if (n == 1) {
assembly {
sstore(slot, or(or(shl(208, lastTimestamp), shl(248, n)), shl(104, shr(152, mload(add(reserves, 32))))))
}
//return;
return (counter_even, counter_odd); /// @audit added for PoC/testing purposes
}
assembly {
sstore(
slot,
or(
or(shl(208, lastTimestamp), shl(248, n)),
or(shl(104, shr(152, mload(add(reserves, 32)))), shr(152, mload(add(reserves, 64))))
)
)
// slot := add(slot, 32)
}
if (n > 2) {
uint256 maxI = n / 2; // number of fully-packed slots
uint256 iByte; // byte offset of the current reserve
counter_even = 0; /// @audit added for PoC/testing purposes
for (uint256 i = 1; i < maxI; ++i) { /// @audit NOT a bug!
//for (uint256 i = 1; i <= maxI; ++i) { /// @audit added for PoC/testing purposes
counter_even = counter_even + 1; /// @audit added for PoC/testing purposes
iByte = i * 64;
assembly {
sstore(
add(slot, i),
add(mload(add(reserves, add(iByte, 32))), shr(128, mload(add(reserves, add(iByte, 64)))))
)
}
}
// If there is an odd number of reserves, create a slot with the last reserve
// Since `i < maxI` above, the next byte offset `maxI * 64`
// Equivalent to "reserves.length % 2 == 1" but cheaper.
counter_odd = 0; /// @audit added for PoC/testing purposes
if (reserves.length & 1 == 1) {
counter_odd = counter_odd + 1; /// @audit added for PoC/testing purposes
iByte = maxI * 64;
assembly {
sstore(
add(slot, maxI),
add(mload(add(reserves, add(iByte, 32))), shr(128, shl(128, sload(add(slot, maxI)))))
)
}
}
return (counter_even, counter_odd); /// @audit added for PoC/testing purposes
}
}
The modified test function:
/// @dev Store fuzzed reserves, re-read and compare.
function testEmaFuzz_storeAndRead(
uint8 n,
uint40 lastTimestamp,
uint256[NUM_RESERVES_MAX] memory _reserves
) public {
n = 7; /// @audit added for PoC/testing purposes
//vm.assume(n <= NUM_RESERVES_MAX);
// Use the first `n` reserves. Cast uint104 reserves -> uint256
uint256[] memory reserves = new uint256[](n);
for (uint256 i; i < n; i++) {
reserves[i] = _reserves[i];
}
(uint256 _counterEven, uint256 _counterOdd) = RESERVES_STORAGE_SLOT.storeLastReserves(lastTimestamp, reserves);
// Re-read reserves and compare
(uint8 _n, uint40 _lastTimestamp, uint256[] memory reserves2) = RESERVES_STORAGE_SLOT.readLastReserves();
uint8 __n = RESERVES_STORAGE_SLOT.readNumberOfReserves();
assertEq(__n, n, "ByteStorage: n mismatch");
assertEq(_n, n, "ByteStorage: n mismatch");
assertEq(_lastTimestamp, lastTimestamp, "ByteStorage: lastTimestamp mismatch");
for (uint256 i; i < reserves2.length; i++) {
assertApproxEqRelN(reserves2[i], reserves[i], 1);
}
console2.log(n, _n, __n, reserves2.length); /// @audit added for PoC/testing purposes
console2.log(_counterEven, _counterOdd); /// @audit added for PoC/testing purposes
console2.log(2 + (2*_counterEven) + _counterOdd); /// @audit added for PoC/testing purposes
}
PoC test results:
Forge test command used:
forge test --contracts test/libraries/LibLastReserveBytes.t.sol --mt testEmaFuzz_storeAndRead -vvvvv
Case n = 3:
$ forge test --contracts test/libraries/LibLastReserveBytes.t.sol --mt testEmaFuzz_storeAndRead -vvvvv
[⠒] Compiling...
[⠘] Compiling 2 files with 0.8.23
[⠃] Solc 0.8.23 finished in 4.54s
Compiler run successful!
Ran 1 test for test/libraries/LibLastReserveBytes.t.sol:LibLastReserveBytesTest
[PASS] testEmaFuzz_storeAndRead(uint8,uint40,uint256[8]) (runs: 258, μ: 58446, ~: 58805)
Traces:
[58670] LibLastReserveBytesTest::testEmaFuzz_storeAndRead(215, 58, [315, 504, 3477, 6779, 6922, 5192296858534827628530496329220095 [5.192e33], 1757, 365])
├─ [0] console::log(3, 3, 3, 3) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(0, 1) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(3) [staticcall]
│ └─ ← [Stop]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 49.46ms (49.02ms CPU time)
Ran 1 test suite in 67.53ms (49.46ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Case n = 4:
$ forge test --contracts test/libraries/LibLastReserveBytes.t.sol --mt testEmaFuzz_storeAndRead -vvvvv
[⠒] Compiling...
[⠑] Compiling 1 files with 0.8.23
[⠘] Solc 0.8.23 finished in 4.45s
Compiler run successful!
Ran 1 test for test/libraries/LibLastReserveBytes.t.sol:LibLastReserveBytesTest
[PASS] testEmaFuzz_storeAndRead(uint8,uint40,uint256[8]) (runs: 258, μ: 61422, ~: 61476)
Traces:
[61572] LibLastReserveBytesTest::testEmaFuzz_storeAndRead(2, 11438916 [1.143e7], [106150193239837258513847 [1.061e23], 38591594367475865826830338579092558716968920492 [3.859e46], 19055696957076428420292313381975300752169703956443552993 [1.905e55], 341419703651992786261262745366373965132438 [3.414e41], 26230693783876097947213092663688534030704545718679730410 [2.623e55], 55832 [5.583e4], 0, 1628216 [1.628e6]])
├─ [0] console::log(4, 4, 4, 4) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(1, 0) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(4) [staticcall]
│ └─ ← [Stop]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 53.36ms (52.88ms CPU time)
Ran 1 test suite in 65.78ms (53.36ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Case n = 5:
$ forge test --contracts test/libraries/LibLastReserveBytes.t.sol --mt testEmaFuzz_storeAndRead -vvvvv
[⠒] Compiling...
[⠘] Compiling 1 files with 0.8.23
[⠃] Solc 0.8.23 finished in 4.53s
Compiler run successful!
Ran 1 test for test/libraries/LibLastReserveBytes.t.sol:LibLastReserveBytesTest
[PASS] testEmaFuzz_storeAndRead(uint8,uint40,uint256[8]) (runs: 258, μ: 86309, ~: 86416)
Traces:
[86356] LibLastReserveBytesTest::testEmaFuzz_storeAndRead(26, 8995830 [8.995e6], [437499694678062113875 [4.374e20], 1307407596014970244126832602014 [1.307e30], 1947529475366674528761523019230759537901934946165 [1.947e48], 64024459532983150427 [6.402e19], 73523514728886635250545 [7.352e22], 9266101179854536768090739732735435690246332740562850978270556758 [9.266e63], 3108862747722808287896380166712983 [3.108e33], 44894599922128458914044733063157480366503274044300842988831653623045586 [4.489e70]])
├─ [0] console::log(5, 5, 5, 5) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(1, 1) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(5) [staticcall]
│ └─ ← [Stop]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 56.78ms (56.45ms CPU time)
Ran 1 test suite in 69.07ms (56.78ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Case n = 2:
$ forge test --contracts test/libraries/LibLastReserveBytes.t.sol --mt testEmaFuzz_storeAndRead -vvvvv
[⠒] Compiling...
[⠒] Compiling 1 files with 0.8.23
[⠑] Solc 0.8.23 finished in 4.44s
Compiler run successful!
Ran 1 test for test/libraries/LibLastReserveBytes.t.sol:LibLastReserveBytesTest
[PASS] testEmaFuzz_storeAndRead(uint8,uint40,uint256[8]) (runs: 258, μ: 33555, ~: 33573)
Traces:
[33597] LibLastReserveBytesTest::testEmaFuzz_storeAndRead(5, 7071, [3789478110522689809460 [3.789e21], 740833129118970771308845931135594206814774089 [7.408e44], 260428098446930484 [2.604e17], 9757737301656387220480213633201549855061368910 [9.757e45], 9701505909937800514171079266333300567381507632893219644199561110782125636 [9.701e72], 12333396240278346886081740464864770281508125439317161104038 [1.233e58], 4330492373958729679170671914402992348671967054905156933135640764999511682718 [4.33e75], 939979624077250503004286003617720900186919569608097335109442101962544 [9.399e68]])
├─ [0] console::log(2, 2, 2, 2) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(0, 0) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(2) [staticcall]
│ └─ ← [Stop]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 38.38ms (37.92ms CPU time)
Ran 1 test suite in 51.95ms (38.38ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Case n = 1:
$ forge test --contracts test/libraries/LibLastReserveBytes.t.sol --mt testEmaFuzz_storeAndRead -vvvvv
[⠒] Compiling...
[⠑] Compiling 1 files with 0.8.23
[⠘] Solc 0.8.23 finished in 4.52s
Compiler run successful!
Ran 1 test for test/libraries/LibLastReserveBytes.t.sol:LibLastReserveBytesTest
[PASS] testEmaFuzz_storeAndRead(uint8,uint40,uint256[8]) (runs: 258, μ: 31116, ~: 31120)
Traces:
[31102] LibLastReserveBytesTest::testEmaFuzz_storeAndRead(109, 138973374830 [1.389e11], [4036, 1599, 3961, 3481, 2180, 3124842405 [3.124e9], 4590, 2275])
├─ [0] console::log(1, 1, 1, 1) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(0, 0) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(2) [staticcall]
│ └─ ← [Stop]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 31.93ms (31.50ms CPU time)
Ran 1 test suite in 47.54ms (31.93ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Ignore the console::log(2) here, it's incorrect, I didnt bother to adapt it for this specific test or the next one.
Case n = 0:
$ forge test --contracts test/libraries/LibLastReserveBytes.t.sol --mt testEmaFuzz_storeAndRead -vvvvv
[⠒] Compiling...
[⠆] Compiling 1 files with 0.8.23
[⠰] Solc 0.8.23 finished in 5.09s
Compiler run successful!
Ran 1 test for test/libraries/LibLastReserveBytes.t.sol:LibLastReserveBytesTest
[PASS] testEmaFuzz_storeAndRead(uint8,uint40,uint256[8]) (runs: 258, μ: 27458, ~: 28615)
Traces:
[28615] LibLastReserveBytesTest::testEmaFuzz_storeAndRead(172, 206840 [2.068e5], [3451, 3076, 1020, 3796, 3085, 4052, 645326474426547203313410069153905908525362434350 [6.453e47], 1293])
├─ [0] console::log(0, 0, 0, 0) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(0, 0) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log(2) [staticcall]
│ └─ ← [Stop]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 38.57ms (38.13ms CPU time)
Ran 1 test suite in 57.54ms (38.57ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Updates
Lead Judging Commences
giovannidisiena about 1 year ago
Submission Judgement Published Reason: Non-acceptable severity
Assigned finding tags:
Informational/Invalid
Prize pool breakdown
Total prize
12,000 USDC
nSLOC
47925 USDC / LOC
10,000 USDC
1,000 USDC
1,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.