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

A validator oracle finalizing a validation would almost always fail

z3r0

Summary

A validator oracle would never be able to validate a completed taskID if its validation scores for each generation are not all the same (which is very unlikely).

Vulnerability Details

A validator calls LLMOracleCoordinator::validate() to validate requests for a given taskId, and if its completed - the validation scores are finalized with a call to LLMOracleCoordinator::finalizeValidation().

Within this function, the mean and standard deviation are computed with a call to Statistics::stddev(). Here is the extract of the function:

...
/// @notice Compute the variance of the data.
/// @param data The data to compute the variance for.
function variance(uint256[] memory data) internal pure returns (uint256 ans, uint256 mean) {
mean = avg(data);
uint256 sum = 0;
for (uint256 i = 0; i < data.length; i++) {
uint256 diff = data[i] - mean; // @audit: would fail for data that falls below the mean e.g mean [7, 5, 12, 10] = 8, 5 - 8 would fail and revert always as the lowest number is always lower than mean except all numbers are the same
sum += diff * diff;
}
ans = sum / data.length;
}
​
/// @notice Compute the standard deviation of the data.
/// @dev Computes variance, and takes the square root.
/// @param data The data to compute the standard deviation for.
function stddev(uint256[] memory data) internal pure returns (uint256 ans, uint256 mean) {
(uint256 _variance, uint256 _mean) = variance(data);
mean = _mean;
ans = sqrt(_variance);
}
...

Within the stddev() function, variance() is called, but this function has a bug when calculating the diff between the current number in the array and the mean of the array:

uint256 diff = data[i] - mean; // @audit: would fail for data that falls below the mean e.g mean [7, 5, 12, 10] = 8, 5 - 8 would fail and revert always as the lowest number is always lower than mean except all numbers are the same

As my comment explains, the difference between the current number in the iteration and the mean is calculated - but this is done wrongly as an underflow is certain to occur as long as the numbers/scores are not all the same e.g mean of numbers [7, 5, 12, 10] = 8.5 (rounded to 8 in solidity), by calculating 5 - 8 this way - an underflow error occurs which causes the entire operation to revert.

This makes it impossible for a validator to finalize scores for a taskID. The only way to avoid this error is to grade all scores for each generation with the same number, but this is very much counterintuitive as it's unreasonable to grade all responses the same - as even completely wrong answers would be graded the same as the best answer.

Impact

DOS for a validator attempting to finalize a validation.

Tools Used

Manual && Solidity

Recommendations

Cast the number and mean to int256 to allow underflow i.e 5 - 8 = -3, and get the absolute number

/// @notice Compute the variance of the data.
/// @param data The data to compute the variance for.
function variance(uint256[] memory data) public returns (uint256 ans, uint256 mean) {
mean = avg(data);
uint256 sum = 0;
for (uint256 i = 0; i < data.length; i++) {
uint256 diff = abs(int256(data[i]) - int256(mean)); // @audit-fix
sum += diff * diff;
}
ans = sum / data.length;
}
​
...
​
function abs(int256 num) internal returns (uint256) { // @audit-fix
return num > 0 ? uint256(num) : uint256(-num);
}
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 9 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.