The LLMOracleCoordinator::assertValidNonce function uses abi.encodePacked to concatenate several values, including taskId, task.input, task.requester, msg.sender, and nonce, into a single byte array for hashing. However, this approach can lead to hash collisions because abi.encodePacked concatenates values directly, potentially causing data overlap when dynamic types are involved. Since task.input is a bytes (dynamic) type and task.requester is an address, a carefully crafted input could result in a hash collision, allowing a malicious actor to bypass the nonce check.
The LLMOracleCoordinator::assertValidNonce function checks the nonce within the LLMOracleCoordinator::response function to verify if a task has been executed by msg.sender. However, the use of abi.encodePacked for encoding can lead to hash collisions.
Here’s how abi.encodePacked operates: it does not pad inputs and does not conform to the full ABI specification. Instead, it encodes parameters using only the minimum bytes required by each type, concatenating them directly. For dynamic types, abi.encodePacked encodes values in-place without including their length, omitting the length value typically appended to the end of encoded data. Additionally, abi.encodePacked does not support encoding for structs or nested arrays.
The function uses abi.encodePacked, which does not provide a separator between concatenated values. This can lead to ambiguity when dynamic types like bytes and structs are involved, as the exact end of one value and start of another may be indistinguishable, creating the possibility for input manipulation.
If a malicious actor carefully constructs a task.input (originating from struct data) that overlaps with task.requester (also from a struct) or msg.sender, the resulting keccak256 hash might still fall within the valid range, allowing them to bypass the nonce validation.
When a generator invokes the LLMOracleCoordinator::response function, providing taskId, nonce, output, and metadata, the task data originates from a struct called TaskRequest within the function. Consequently, the function calls LLMOracleCoordinator::assertValidNonce, an internal function that manages hashing. However, since the data in the struct could overlap, and abi.encodePacked does not fully support structs, this could lead to hash collision.
The resulting keccak256 hash might still fall within the acceptable range, allowing the nonce validation to be bypassed.
This vulnerability could allow a malicious actor to generate hash collisions, bypassing nonce validation and gaining unauthorized access to actions restricted by the nonce. This bypass could lead to unintended task processing and manipulation of task states.
Manuel review
To prevent hash collisions, replace abi.encodePacked with abi.encode, which provides a more explicit and unambiguous encoding with padding and length fields. This will ensure that each variable in message remains distinct, eliminating the risk of concatenation-based hash collisions.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.