Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing validations #2125

Closed
HDauven opened this issue Aug 14, 2024 · 2 comments · Fixed by #2157
Closed

Missing validations #2125

HDauven opened this issue Aug 14, 2024 · 2 comments · Fixed by #2157
Assignees
Labels
module:consensus Issues related to consensus module module:node Issues related to node module module:node-data Issues related to node-data module type:enhancement Issues concerning code or feature improvement (performance, refactoring, etc)

Comments

@HDauven
Copy link
Member

HDauven commented Aug 14, 2024

Summary

  1. In node/src/network/frame.rs:25, field checksum is declared. It is initialized on line 35, but it is never utilized to verify a header consistency.
  2. In node-data/src/ledger/attestation.rs:35, the structure StepVotes is considered to be empty if any of bitset or aggregate_signature is zero. However, if only one of these fields is zero then it must be treated as an inconsistent structure.
  3. In node/src/databroker.rs:303, a child block was added to the inventory. This is part of the traversal from one block specified by its hash through all of its descendants. To ensure consistency of the database, the value of prev_block_hash of each child pair should be checked against the hash of its parent.
  4. Within consensus/src/user/provisioners.rs:255-263, edge case of a single active provisioner is handled. Set members should be checked to be non-empty in the end. It can be useful to double-check that exclusion set is equal to eligibles set in this edge case.
  5. In node-data/src/ledger.rs:38, the function to_str returns an error because the length of the hexadecimal string is not even. However, the function hex::encode called on line 36 always returns even strings.

Solution

We recommend adding validation checks and utilizing debug_assert! for invariants checks.

@HDauven HDauven added type:enhancement Issues concerning code or feature improvement (performance, refactoring, etc) module:consensus Issues related to consensus module module:node Issues related to node module module:node-data Issues related to node-data module labels Aug 14, 2024
@goshawk-3
Copy link
Contributor

goshawk-3 commented Aug 14, 2024

In node/src/network/frame.rs:25, field checksum is declared. It is initialized on line 35, but it is never utilized to verify a header consistency.

Resolved in 16e18ae

In node-data/src/ledger.rs:38, the function to_str returns an error because the length of the hexadecimal string is not even. However, the function hex::encode called on line 36 always returns even strings.

Resolved in #2088

@HDauven HDauven added this to Audits Aug 16, 2024
@HDauven HDauven moved this to In Progress in Audits Aug 16, 2024
@autholykos
Copy link
Member

@goshawk-3 can you please merge #2157 so we can close this one?

@github-project-automation github-project-automation bot moved this from In Review to Done in Audits Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:consensus Issues related to consensus module module:node Issues related to node module module:node-data Issues related to node-data module type:enhancement Issues concerning code or feature improvement (performance, refactoring, etc)
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants