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

Lack of output validation in `LLMOracleCoordinator::respond` allows empty responses and potential fee exploitation by oracles.

v1vah0us3

Summary

The LLMOracleCoordinator::respond function does not validate the contents of the output parameter, allowing registered generators to submit empty responses. This lack of validation can result in incomplete or unusable outputs stored in the TaskResponse array of structs. If numValidations is set to 0, meaning no validation phase occurs, the response generator receives the generatorFee without any content checks, leading to potential exploitation of the fee system.

Description

In the respond function, generators submit responses containing output data. However, there is no mechanism to ensure that the output is not empty, even though natspec requires it: @dev output must not be empty. As such, it is possible for a registered generator to submit broken or empty outputs.
The lack of output validation is further compounded when task.parameters.numValidations is set to 0, meaning no validation phase occurs. Under this condition:

  1. There are no checks to ensure that the output is non-empty or meets minimal quality criteria.

  2. When task.parameters.numValidations == 0, the task’s status is set to Completed immediately, bypassing the validation phase. This results in direct rewards for generators without any verification of the output’s quality or relevance.

  3. If the generatorFee is substantial, this design could be exploited by malicious actors who submit arbitrary or empty outputs to repeatedly collect fees without providing meaningful contributions. While the assertValidNonce function’s Proof-of-Work mechanism requires computational effort from oracles, it does not ensure that the content is correct or complete, leaving some potential for abuse by dishonest oracles.

On the other hand, if numValidations > 0, only those respondents whose answers achieve an "above-average" score will receive a fee, see [https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/llm/LLMOracleCoordinator.sol#L368-L369].

PoC

Here is a valid test case that serves as a proof of concept, please paste it into the LLMOracleCoordinator.test.ts file:

describe("Zero Response", function () {
const [numGenerations, numValidations] = [2, 0];
let generatorAllowancesBefore: bigint[];
this.beforeAll(async () => {
taskId++;
generatorAllowancesBefore = await Promise.all(
generators.slice(0, 2).map((g) => token.allowance(coordinatorAddress, g.address))
);
});
it("should allow both generators to respond with an empty output and receive their respective fees", async function () {
const input = "0x" + Buffer.from("What is 2 + 2?").toString("hex"); // Valid input for a new task
const emptyOutput = "0x"; // empty output
const emptyMetadata = "0x"; // empty metadata
await safeRequest(coordinator, token, requester, taskId, input, models, {
difficulty,
numGenerations,
numValidations,
});
const allowancesBefore = await Promise.all(
generators.slice(0, 2).map((g) => token.allowance(coordinatorAddress, g.address))
);
await safeRespond(coordinator, generators[0], emptyOutput, emptyMetadata, taskId, 0n);
await safeRespond(coordinator, generators[1], emptyOutput, emptyMetadata, taskId, 1n);
​
const responses = await coordinator.getResponses(taskId);
console.log("Generator oracle responses after empty output submissions with no validation required:");
responses.forEach((response, index) => {
console.log(` Generator ${index + 1} Address: ${response.responder}, Score: ${response.score}, Output: ${response.output}`);
});
// Check that both responses are empty outputs
responses.forEach((response) => {
expect(response.output).to.equal(emptyOutput);
});
// Capture allowances after responses and calculate fees received
const allowancesAfter = await Promise.all(
generators.slice(0, 2).map((g) => token.allowance(coordinatorAddress, g.address))
);
// Check allowances and log fee information for each generator
allowancesAfter.forEach((allowanceAfter, i) => {
const allowanceBefore = allowancesBefore[i];
const feeReceived = allowanceAfter - allowanceBefore;
console.log(`Allowance before response for Generator ${i + 1}: ${allowanceBefore}`);
console.log(`Allowance after response for Generator ${i + 1}: ${allowanceAfter}`);
console.log(`Fee received by Generator ${i + 1}: ${feeReceived}`);
expect(allowanceAfter).to.be.above(allowanceBefore);
});
});
});

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

Generator oracle responses after empty output submissions with no validation required:
Generator 1 Address: 0x90F79bf6EB2c4f870365E785982E1f101E93b906, Score: 0, Output: 0x
Generator 2 Address: 0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65, Score: 0, Output: 0x
Allowance before response for Generator 1: 0
Allowance after response for Generator 1: 16000000000000000
Fee received by Generator 1: 16000000000000000
Allowance before response for Generator 2: 0
Allowance after response for Generator 2: 16000000000000000
Fee received by Generator 2: 16000000000000000

As shown in the error logs, both oracle responses contain an empty output with a score of 0, which is expected since no validation was required in this case. Both generators successfully received their respective fees for responding. Note that in this scenario, the protocol accepts empty outputs as valid responses.

Impact

Without required response validations, generators can earn fees for responses that may be empty or irrelevant. This can lead to potential fee drift and higher costs for users without providing meaningful value in return.

Tools Used

Manual Code Review, Hardhat

Recommendations

In LLMOracleCoordinator::respond, consider implementing checks to at least ensure that the output field is not empty and meets minimum length criteria.

+ // Define the minimum required length for output in LLMOracleCoordinator.sol
+ uint256 public constant MIN_OUTPUT_LENGTH = 16;
​
function respond(uint256 taskId, uint256 nonce, bytes calldata output, bytes calldata metadata)
public
onlyRegistered(LLMOracleKind.Generator)
onlyAtStatus(taskId, TaskStatus.PendingGeneration)
{
TaskRequest storage task = requests[taskId];
​
+ // Check that the output is not empty and meets minimal length requirement
+ require(output.length >= MIN_OUTPUT_LENGTH, "InvalidOutput: Empty or too short");
​
.............
.............
}

If possible, introduce a minimum validation requirement in LLMOracleManager.sol, such as task.parameters.numValidations == 1, to ensure that tasks undergo some scrutiny before reaching the Completed status, and before any generator fees are distributed.

Updates

Lead Judging Commences

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

Incomplete checks in `respond()` of `LLMOracleCoordinator.sol`, `output` is not checked

Support

FAQs

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