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

Misleading Documentation/NatSpec Comments

johny7173

Summary

Several instances of incorrect or misleading documentation/NatSpec comments have been identified across the protocol's codebase. These inaccuracies can cause confusion among developers, auditors, and users, leading to potential misunderstandings, improper use of the contracts, and overlooked vulnerabilities. Accurate documentation is crucial for the secure and correct functioning of smart contracts.

Vulnerability Details

Instance 1: Incorrect @param Description in register Function

In the register function, the @param description incorrectly refers to unregistering an Oracle:

/// @notice Register an Oracle.
/// @dev Reverts if the user is already registered or has insufficient funds.
/// @param kind The kind of Oracle to unregister. // Incorrect, should be 'to register'
function register(LLMOracleKind kind) public {
// Function implementation
}
​

Issue: The @param comment should describe registering, not unregistering, as this is the register function.

Instance 2: Misleading @dev Comment in state Variable Declaration

In the BuyerAgent contract, the documentation for the state variable implies that only the oracle can update it via updateState:

/// @notice State of the buyer agent.
/// @dev Only updated by the oracle via `updateState`.
bytes public state;
​

However, the updateState function is protected by the onlyAuthorized modifier:

modifier onlyAuthorized() {
if (!swan.isOperator(msg.sender) && msg.sender != owner()) {
revert Unauthorized(msg.sender);
}
_;
}
​

This means the function can be called by the owner, operator, or Swan—not exclusively by the oracle.

Issue: The comment misleads readers into thinking only the oracle can update the state, which is not the case.

Instance 3: Reference to Non-Existent Variable in oracleStateRequests Mapping

Again, in the BuyerAgent contract:

/// @notice Oracle requests for each round about buyer state updates.
/// @dev A taskId of 0 means no request has been made.
/// @dev A non-zero taskId means a request has been made, but not necessarily processed.
/// @dev To see if a task is completed, check `isOracleTaskProcessed`. // Incorrect reference
mapping(uint256 round => uint256 taskId) public oracleStateRequests;
​
/// @notice Indicates whether a given task has been processed.
/// @dev This is used to prevent double processing of the same task.
mapping(uint256 taskId => bool isProcessed) public isOracleRequestProcessed;
​

Issue: The comment refers to isOracleTaskProcessed, which doesn't exist. The correct variable is isOracleRequestProcessed.

Impact

Inaccurate or misleading documentation can significantly impact the protocol's security, usability, and reliability. Developers, users and integrators may misinterpret how functions should be used or the access controls in place, leading to incorrect implementations or introducing bugs.

Tools Used

Manual Review

Recommended Mitigation

It is recommended to perform a review of all documentation and NatSpec comments throughout the codebase to identify and correct inaccuracies and inconsistencies. Updating the comments to accurately reflect the code's functionality and access controls will enhance clarity and prevent misunderstandings.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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