Dria

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

Statistics::variance - function will revert due to underflow if mean > data[i]

Summary

Link: https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/libraries/Statistics.sol#L22

Variance should be calculated by squaring the difference between each data point and the mean. However, if mean > data[i], the following line will generate a panic error due to underflow:

uint256 diff = data[i] - mean;

Vulnerability Details

Proof of Concept:

Add the following test file (eg: Statistics.test.ts) to the test folder and run the test by executing:

npx hardhat test --network hardhat test/Statistics.test.ts

import { expect } from "chai";
import { ethers } from "hardhat";
import type { ERC20, LLMOracleCoordinator, LLMOracleRegistry } from "../typechain-types";
import type { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers";
import { parseEther } from "ethers";
import { deployLLMFixture, deployTokenFixture } from "./fixtures/deploy";
import { registerOracles, safeRequest, safeRespond, safeValidate } from "./helpers";
import { transferTokens } from "./helpers";
import { PANIC_CODES } from "@nomicfoundation/hardhat-chai-matchers/panic";
describe("Statistics", function () {
let dria: HardhatEthersSigner;
let requester: HardhatEthersSigner;
let generators: HardhatEthersSigner[];
let validators: HardhatEthersSigner[];
let coordinator: LLMOracleCoordinator;
let registry: LLMOracleRegistry;
let token: ERC20;
let taskId = 1n;
const input = "0x" + Buffer.from("What is 2 + 2?").toString("hex");
const output = "0x" + Buffer.from("2 + 2 equals 4.").toString("hex");
const models = "0x" + Buffer.from("gpt-4o-mini").toString("hex");
const metadata = "0x";
const difficulty = 2;
const SUPPLY = parseEther("1000");
const STAKES = {
generatorStakeAmount: parseEther("0.01"),
validatorStakeAmount: parseEther("0.01"),
};
const FEES = {
platformFee: parseEther("0.001"),
generationFee: parseEther("0.002"),
validationFee: parseEther("0.0003"),
};
const [numGenerations, numValidations] = [2, 2];
this.beforeAll(async function () {
const [deployer, req1, gen1, gen2, gen3, val1, val2] = await ethers.getSigners();
dria = deployer;
requester = req1;
generators = [gen1, gen2, gen3];
validators = [val1, val2];
token = await deployTokenFixture(deployer, SUPPLY);
({ registry, coordinator } = await deployLLMFixture(dria, token, STAKES, FEES));
const requesterFunds = parseEther("1");
await transferTokens(token, [
[requester.address, requesterFunds],
...generators.map<[string, bigint]>((oracle) => [oracle.address, STAKES.generatorStakeAmount]),
...validators.map<[string, bigint]>((oracle) => [oracle.address, STAKES.validatorStakeAmount]),
]);
});
it("Variance function in Statistics library underflows", async function () {
await registerOracles(token, registry, generators, validators, STAKES);
//make a request
await safeRequest(coordinator, token, requester, taskId, input, models, {
difficulty,
numGenerations,
numValidations,
});
//respond
for (let i = 0; i < numGenerations; i++) {
await safeRespond(coordinator, generators[i], output, metadata, taskId, BigInt(i));
}
//validate
let scores = new Array(6n, 4n)
await safeValidate(coordinator, validators[0], scores, metadata, taskId, 0n);
//second validation => calls finalizeValidation => calls Statistics.stddev(generationScores)
scores = new Array(9n, 7n)
//this will generate an underflow by calculating: uint256 diff = data[i] - mean; in Statistics::variance =>
//uint256 diff = 6 - 7 => underflow !
await expect(safeValidate(coordinator, validators[1], scores, metadata, taskId, 1n))
.to.be.revertedWithPanic(PANIC_CODES.ARITHMETIC_OVERFLOW);
});
});

Impact

The faulty Statistics::variance function is called by Statistics::stddev, which is called by the private LLMOracleCoordinator::finalizeValidation function, which in turn is called by LLMOracleCoordinator::validate

This function is required to validate requests for a given taskId. Because of the vulnerability, most validations will fail, which prevents the protocol from functioning properly.

Tools Used

Manual Review

Recommendations

Modify the variance function

function variance(uint256[] memory data) internal pure returns (uint256 ans, uint256 mean) {
+ require(data.length > 0, "Data cannot be empty");
mean = avg(data);
uint256 sum = 0;
for (uint256 i = 0; i < data.length; i++) {
+ int256 diff = int256(data[i]) - int256(mean);
+ sum += uint256(diff * diff);
- uint256 diff = data[i] - mean;
- sum += diff * diff;
}
ans = sum / 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.