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

Security: Replace older duplicate queued checkpoint blocks with the latest block's data #2697

Merged
merged 2 commits into from
Aug 31, 2021

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Aug 30, 2021

Motivation

We don't check the authorizing data hash until checkpoint blocks reach the state.

So signatures, proofs, or scripts could be different, even if the block hash is the same.

This is unexpected work in sprint 17.

Specifications

https://zips.z.cash/zip-0244

Solution

  • Replace the data from the old block, even if the block header hashes match

We'll write tests for these kinds of block modifications in #2559.

Review

@conradoplg worked on the authorization hash security design.
This fix isn't urgent.

This PR is based on PR #2696, so it might need a rebase after that PR merges.

Reviewer Checklist

  • Code is more secure
  • Existing tests pass

@teor2345 teor2345 added C-bug Category: This is a bug A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code NU-5 Network Upgrade: NU5 specific tasks P-Medium C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data labels Aug 30, 2021
@teor2345 teor2345 added this to the 2021 Sprint 17 milestone Aug 30, 2021
@teor2345 teor2345 requested a review from conradoplg August 30, 2021 10:17
@teor2345 teor2345 self-assigned this Aug 30, 2021
@teor2345 teor2345 force-pushed the checkpoint-verifier-efficient branch from 850b9ed to 52cef8c Compare August 30, 2021 10:19
@teor2345 teor2345 force-pushed the replace-duplicate-checkpoint-block branch from 4ac4984 to c17baba Compare August 30, 2021 10:21
@teor2345 teor2345 changed the title Security: Replace queued checkpoint blocks with duplicate hashes Security: Replace the entire queued checkpoint block if it has a duplicate hash Aug 30, 2021
@teor2345 teor2345 changed the title Security: Replace the entire queued checkpoint block if it has a duplicate hash Security: Replace older duplicate queued checkpoint blocks with the latest block's data Aug 30, 2021
@teor2345 teor2345 force-pushed the checkpoint-verifier-efficient branch from 14dde60 to 279b79f Compare August 30, 2021 23:41
@teor2345 teor2345 force-pushed the replace-duplicate-checkpoint-block branch from c17baba to 0adb91f Compare August 30, 2021 23:43
Base automatically changed from checkpoint-verifier-efficient to main August 31, 2021 00:55
We don't check the authorizing data hash until checkpoint blocks reach the state.

So signatures, proofs, or scripts could be different,
even if the block hash is the same.
@teor2345 teor2345 force-pushed the replace-duplicate-checkpoint-block branch from 0adb91f to 2bdbbc0 Compare August 31, 2021 00:57
@teor2345
Copy link
Contributor Author

The cache action did not output anything, and was cancelled after 30 minutes:
https://github.com/ZcashFoundation/zebra/pull/2697/checks?check_run_id=3467920379

@conradoplg conradoplg merged commit 34c7a27 into main Aug 31, 2021
@conradoplg conradoplg deleted the replace-duplicate-checkpoint-block branch August 31, 2021 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-bug Category: This is a bug C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data NU-5 Network Upgrade: NU5 specific tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants