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

It is fairly possible that the Statistics::variance() function reverts due to underflow

dimulski

Summary

The Swan protocol calculates mean, variance and standard deviation in order to account for variability of the oracle's responses. However the way the variance is calculated doesn't account for all cases. The variance() function doesn't consider the case where the mean can be bigger than a certain score a validator provided:

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;
sum += diff * diff;
}
ans = sum / data.length;
}

From Investopedia: Variance is calculated by taking the differences between each number in the data set and the mean, squaring the differences to make them positive, and then dividing the sum of the squares by the number of values in the data set.
As can be seen from the code snippet above if a number from the data array is smaller than the calculated mean, the function will revert. This will results in the generators and validators who already performed the work to generate and validate for a certain task not getting paid. As well as the buyer agent won't be able to purchase any NFTs in the round he requested his task to be generated and validated. He already paid fees when the request() function was called, those fees will be lost. When there are more validators the chance of this issue happening increases. For example if there are only 2 validators(expected values for the score are from 0 to 1e18) if the difference between the two values provided by the validators is 2 wei, then the variance() function will revert.

Vulnerability Details

Gist

After following the steps in the above mentioned gist add the following steps to the AuditorTests.t.sol file:

function test_VarianceCalculationsReverts() public {
vm.startPrank(owner);
LLMOracleTaskParameters memory swanOracleParametetrsNew = LLMOracleTaskParameters({
difficulty: uint8(1),
numGenerations: uint40(1),
numValidations: uint40(3)
});
swanInstance.setOracleParameters(swanOracleParametetrsNew);
vm.stopPrank();
​
vm.startPrank(alice);
BuyerAgent buyerAgentAlice;
buyerAgentAlice = swanInstance.createBuyer("BigSpender", "SpendBig", uint96(1), 1_000e18);
mockERC20.mint(alice, 1_000e18);
mockERC20.transfer(address(buyerAgentAlice), 1_000e18);
vm.stopPrank();
​
​
vm.startPrank(bob);
swanInstance.list("Name", "Symbol", "perfectForSimulation", 0, address(buyerAgentAlice));
address[] memory createdNFTs = new address[]();
createdNFTs = swanInstance.getListedAssets(address(buyerAgentAlice), 0);
vm.stopPrank();
​
vm.startPrank(alice);
skip(3601);
buyerAgentAlice.oraclePurchaseRequest("0xsomething", "0xsomething");
vm.stopPrank();
​
vm.startPrank(generator);
bytes memory output = abi.encode(createdNFTs);
uint256 taskId = buyerAgentAlice.oraclePurchaseRequests(0);
LLMOracleCoordinatorInstance.respond(taskId, 1, output, "");
vm.stopPrank();
​
vm.startPrank(validator);
uint256[] memory scoresValidator = new uint256[]();
scoresValidator[0] = 0.8e18;
LLMOracleCoordinatorInstance.validate(taskId, 1e18, scoresValidator, "");
vm.stopPrank();
​
vm.startPrank(validator1);
LLMOracleCoordinatorInstance.validate(taskId, 1e18, scoresValidator, "");
vm.stopPrank();
​
vm.startPrank(validator2);
uint256[] memory scoresValidator2 = new uint256[]();
scoresValidator2[0] = 0.7e18;
vm.expectRevert(stdError.arithmeticError);
LLMOracleCoordinatorInstance.validate(taskId, 1e18, scoresValidator2, "");
vm.stopPrank();
}

To run the test use: forge test -vvv --mt test_VarianceCalculationsReverts

Impact

There are a couple of impacts. This vulnerability will results in the generators and validators who already performed the work to generate and validate for a certain task not getting paid. As well as the buyer agent won't be able to purchase any NFTs in the round he requested his task to be generated and validated for. He already paid fees when the request() function was called. So validators, generators and the buyer agent all loose fees. Additionally, given the fact that the only purpose of the protocol is to allow buyer agent to buy Swan NFTs so they can proceed their simulation this vulnerability makes the whole protocol obsolete.

Tools Used

Manual review & Foundry

Recommendations

Consider checking whether the number from the data array is less than the mean, and if so subtract that number from the mean, then square the diff and add it to the sum.

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.