Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Arithmetic Underflow in `Statistics::variance` function causes Revert

vasquez

Summary

The Statistics::variance function causes arithmetic underflow when input data contains values smaller than the mean. This results in a revert, breaking contract functionality.

Vulnerability Details

The issue lies in the variance calculation. In the loop where the mean is subtracted from each number in the input array (uint256 diff = data[i] - mean;), if any number(data[i]) is smaller than the mean, the subtraction underflows, causing the function to revert. This is problematic because uint256 is unsigned, meaning it cannot represent negative numbers.

POC

Add this test in a .t.sol file in the test directory.
(For foundry setup in hardhat project: Article)

// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.20;
​
import "forge-std/Test.sol";
import "../contracts/libraries/Statistics.sol";
​
contract StatisticsTest is Test {
using Statistics for uint256[];
​
function testVarianceWithMixedValues() public {
uint256[] memory data = new uint256[]();
data[0] = 1;
data[1] = 2;
data[2] = 3;
data[3] = 4;
data[4] = 5;
(uint256 variance, uint256 mean) = data.variance();
assertEq(variance, 2, "Variance should be 2 for sequence 1 to 5");
assertEq(mean, 3, "Mean should be 3 for sequence 1 to 5");
}

forge test --mt testVarianceWithMixedValues -vvvvv , results in:

Ran 1 test for test/StatisticsTest.t.sol:StatisticsTest
[FAIL: panic: arithmetic underflow or overflow (0x11)] testVarianceWithMixedValues() (gas: 1880)
Traces:
[1880] StatisticsTest::testVarianceWithMixedValues()
└─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
​
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 6.37ms (1.41ms CPU time)
​
Ran 1 test suite in 239.51ms (6.37ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
​
Failing tests:
Encountered 1 failing test in test/StatisticsTest.t.sol:StatisticsTest
[FAIL: panic: arithmetic underflow or overflow (0x11)] testVarianceWithMixedValues() (gas: 1880)
​
Encountered a total of 1 failing tests, 0 tests succeeded

Impact

LLMOracleCoordinator will be unable to provide any results, braking the BuyerAgent functionalities, making the protocol almost unusable.

Tools Used

Manual Review.

Recommendations

Update some of the values to int256:

function variance(uint256[] memory data) internal pure returns (uint256 ans, uint256 mean) {
mean = avg(data);
- uint256 sum = 0;
+ int256 sum = 0;
for (uint256 i = 0; i < data.length; i++) {
- uint256 diff = data[i] - mean;
+ int256 diff = data[i] - mean;
sum += diff * diff;
}
- ans = sum / data.length;
+ ans = uint256(sum / int256(data.length));
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Underflow in computing variance

Support

FAQs

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