fix(identity): stop accepting forged zero-knowledge proofs - #1078
Open
jonathanayubausara-a11y wants to merge 1 commit into
Open
fix(identity): stop accepting forged zero-knowledge proofs#1078jonathanayubausara-a11y wants to merge 1 commit into
jonathanayubausara-a11y wants to merge 1 commit into
Conversation
verify_privacy_preserving previously "verified" proofs with a length check (any 32-byte proof with verification_type "identity_proof" passed), so a forged proof reported success, bumped the privacy nonce, and updated observable state. On-chain zero-knowledge proof verification is not implemented in this contract, so the message now always fails with ZeroKnowledgeVerificationUnsupported instead of pretending to verify; the fake verify_zero_knowledge_proof helper is removed and the direction is documented on the message. Closes MettaChain#1023
|
@jonathanayubausara-a11y Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #1023.
verify_zero_knowledge_proofwas a length check: any 32-byteproofwithverification_type == "identity_proof"passed, so a forged proof reported success, bumped the privacy nonce, and updated observablelast_activitystate — whilePrivacyVerificationFailedimplied real proof verification.Per the issue's stated scope (making the behavior honest and safe; real ZK integration is out of scope), the chosen direction is an explicit not-implemented error:
verify_privacy_preservingnow always fails with the newZeroKnowledgeVerificationUnsupportederror once the caller has an identity with ZK privacy enabled — it never reports success for an unverified proof, so no nonce bump or state update can be triggered by a forged proof.verify_zero_knowledge_proofhelper is removed.Verification
cargo test -p propchain-identity— 25 passed, including the updated privacy-preserving verification test asserting a 32-byte forged proof is rejectedcargo clippy -p propchain-identity --all-targets -- -D warnings— cleancargo check --workspace— cleanCloses #1023