fix: replace assert with raise ValueError in crypto-critical code#811
Open
tombudd wants to merge 1 commit intoElection-Tech-Initiative:mainfrom
Open
fix: replace assert with raise ValueError in crypto-critical code#811tombudd wants to merge 1 commit intoElection-Tech-Initiative:mainfrom
tombudd wants to merge 1 commit intoElection-Tech-Initiative:mainfrom
Conversation
Python assert statements are silently stripped when running with the -O (optimize) flag. In election security software, this means critical validation checks disappear without warning: - utils.py: None check on unwrap could pass through None values - group.py: Zero-value check on multiplicative inverse could allow division-by-zero in modular arithmetic - elgamal.py: Empty ciphertext check could produce undefined behavior in homomorphic accumulation - chaum_pedersen.py: Plaintext range check could allow invalid values in Chaum-Pedersen zero-knowledge proofs All four assert statements guarded cryptographic invariants that must hold regardless of Python optimization level. Replaced with explicit raise ValueError to ensure these checks are never bypassed. Identified and authored by UNA (https://resoverse.io) -- an autonomous AI agent (Governed Digital Organism) designed and built by Tom Budd. Co-Authored-By: UNA <una@resoverse.io>
Author
|
📋 Full review documentation: https://www.notion.so/32c1daab53fa81ef80fde16abcb0c7f3 |
There was a problem hiding this comment.
Pull request overview
This PR hardens election cryptography code paths by replacing assert-based invariant checks (which are removed under Python -O) with explicit runtime exceptions, ensuring the guards remain active in optimized deployments.
Changes:
- Replace
assert optional is not Nonewith an explicitValueErroringet_optional. - Replace
assert e != 0with an explicitValueErrorin modular multiplicative inverse computation. - Replace
assert len(ciphertexts) != 0with an explicitValueErrorin ElGamal homomorphic accumulation. - Replace
assert 0 <= plaintext <= 1with an explicitValueErrorin disjunctive Chaum-Pedersen proof generation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/electionguard/utils.py |
Makes get_optional fail safely under -O by raising ValueError when given None. |
src/electionguard/group.py |
Preserves the zero-inverse guard at runtime by raising ValueError for e == 0. |
src/electionguard/elgamal.py |
Ensures elgamal_add() always enforces non-empty inputs regardless of optimization flags. |
src/electionguard/chaum_pedersen.py |
Ensures plaintext domain validation (0/1) is enforced at runtime with ValueError. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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
Four
assertstatements in crypto-critical code paths guard invariants that must hold for election integrity. Python silently strips allassertstatements when running with the-O(optimize) flag, meaning these safety checks disappear without warning in optimized deployments.The Problem
When running
python -O(which is common in production deployments for performance), the following checks are completely removed:utils.pyassert optional is not Nonegroup.pyassert e != 0elgamal.pyassert len(ciphertexts) != 0chaum_pedersen.pyassert 0 <= plaintext <= 1In election software using homomorphic encryption, silent bypass of these checks could lead to corrupted tallies, invalid proofs, or cryptographic failures that undermine election verifiability.
Changes
Replaced all four
assertstatements with explicitraise ValueErrorthat persists regardless of Python optimization level.About This Review
This vulnerability was identified and fixed by UNA (https://resoverse.io) — an autonomous AI agent (Governed Digital Organism) designed and built by Tom Budd (https://tombudd.com). UNA specializes in open-source code quality, security, and documentation reviews for projects aligned with beneficial AI, democracy, and open-source values.
Interested in having UNA review your codebase? Reach out: tom@tombudd.com | tombudd.com
Review #3 — UNA Open Source Reviews