Dria

Summary

The LLMOracleCoordinator::finalizeValidation function calculates the range for valid scores depending on the result of the expression score >= _mean - _stddev. If _mean is less than _stddev, this calculation leads to an underflow error, causing a revert that will fail the transaction. This behavior prevents successful validation completion and rewards distribution, disrupting normal contract operations and usability.

Description

In the LLMOracleCoordinator::finalizeValidation function, scores are evaluated within a standard deviation range around the mean, using the criteria (_mean - _stddev) and (_mean + _stddev), see [https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/llm/LLMOracleCoordinator.sol#L343]:

However, in cases where _mean < _stddev, such as some valid edge case where for example scores[] = [0,1,0,1,2], the calculation of _mean - _stddev attempts to produce a negative value.

Since Solidity’s uint256 type does not support negative numbers, this results in an underflow, triggering an automatic revert and causing the transaction to fail. The edge case described results in _stddev = 1 and _mean = 0, which causes the check score >= _mean - _stddev to revert, as _mean - _stddev evaluates to a negative result.

The same issue exists also in [https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/llm/LLMOracleCoordinator.sol#L368]:

Exploit Scenario

For the input scores[] = [0, 1, 0, 1, 2], the standard deviation and mean calculated by Statistics.stddev(scores) in LLMOracleCoordinator::finalizeValidation are _stddev = 1 and _mean = 0. Note that the calculation using the Statistics.sol library would be successful in this case. That can be quickly checked in Remix:

When performing the range check in (score >= _mean - _stddev) (see [https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/llm/LLMOracleCoordinator.sol#L343]), the _mean - _stddev calculation attempts to compute 0 - 1, which underflows as it is not representable within the unsigned integer type, triggering a revert. The same problem was also found at [https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/llm/LLMOracleCoordinator.sol#L368].

PoC

For the proof of concept, we will stick to the same score values as shown in the section above. Here is the test case and setup, please paste it into the LLMOracleCoordinator.test.ts file and adjust the this.beforeAll(async function ()) section with additional validators, like this:

The test can be run with yarn test ./test/LLMOracleCoordinator.test.ts --verbose .
Here are the logs:

As you can see from the error logs, the transaction failed with an error message: panic code 0x11 (Arithmetic operation overflowed outside of an unchecked block). And the problem was not the use of a buggy Statistics.sol library, but the logic behind the score range calculation in LLMOracleCoordinator::finalizeValidation.

Impact

Tasks with scores causing _mean < _stddev cannot complete, leading to a halt in the validation process and blocking reward distribution. Since scores submitted by prior validators to the TaskValidation struct array are immutable, tasks may become permanently locked in PendingValidation status, posing a potential DoS vulnerability.

Tools Used

Manual Code Review, Remix, Hardhat

Recommendations

LLMOracleCoordinator::finalizeValidation needs to be refactored. You could think about rewriting both underflow-prone conditions in finalizeValidation using only addition operations, adjusting the logic to avoid subtraction:

After applying the suggested fixes, the issues are mitigated. Adjust the last line in the PoC to expect(finalRequest.status).to.equal(TaskStatus.Completed); and run the test with yarn test ./test/LLMOracleCoordinator.test.ts, it will pass.