Improve test coverage and document possible vulnerabilities (unexploitable under different inter-query keys)#9
Open
p0mvn wants to merge 1 commit intomenonsamir:artifactfrom
Conversation
…ding The `assert!(val < lwe_q_prime)` range checks in `decode_response_normal_yclient` (lines 718-723, 731-736) panic on malicious server responses in a way that depends on the client's secret key. A malicious server can craft responses that cause some keys to panic and others to succeed, leaking 1 bit of key-dependent information per query. This commit adds: - Comprehensive malformed-response test suites for all three decode paths (YClient::decode_response, decode_response_normal_yclient, decode_response_simplepir_yclient) - Two #[ignore]'d tests that demonstrate the selective-failure side channel with concrete observed outcomes - Boundary, overflow, and determinism tests for modulus_switch::recover Made-with: Cursor
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.
Feedback from Zellic audit.
Summary
This PR adds test suites that document two security vulnerabilities in the YPIR client response-decoding path, along with comprehensive boundary/edge-case tests for malformed server responses.
Note: the vulnerabilities are unexploitable under different inter-query keys.
Vulnerability 1 — Selective-failure side channel (
decode_response_normal_yclient)Location:
src/client.rs, lines 718-723 and 731-736The
assert!(val < lwe_q_prime)range checks in the RLWE→LWE decoding pipeline panic when a deserialized coefficient exceedslwe_q_prime. Because the coefficients are derived by subtracting the client's secret key from the ciphertext, whether the assertion fires depends on the secret key.A malicious server can exploit this by crafting a response where:
This leaks 1 bit of secret-key-dependent information per query. The server observes whether the client proceeds normally or crashes/disconnects, distinguishing keys without ever seeing them.
Concrete demonstration: The test
decode_normal_crafted_near_boundary_same_outcomefeeds a0x7F-filled response to 5 different keys and observes outcomes[false, false, true, false, false]— confirming the side channel. A second test (decode_normal_max_value_bytes_same_outcome_different_keys) shows the same divergence on0xFF-filled responses.Suggested fix: Replace the
assert!calls with saturating/wrapping arithmetic or return aResult, ensuring the code path is identical regardless of the secret key.Vulnerability 2 — Unchecked accumulator overflow (
YClient::decode_response)Location:
src/client.rs,decode_responsemethodThe inner-product accumulator uses
u128, but the function does not validate that inputs are< params.modulus. When givenu64::MAXvalues,poly_len × (u64::MAX)²overflowsu128::MAX, causing a panic in debug mode or silent wraparound in release mode.While not directly exploitable as a side channel (the overflow is key-independent), it means a malicious server can cause a guaranteed client crash with crafted responses, and the silent wraparound in release mode produces garbage plaintext.
Suggested fix: Validate input coefficients are
< params.modulusbefore accumulation, or document the precondition and enforce it at the deserialization boundary.What this PR adds
src/client.rsmalformed_response_testsdecode_response,decode_response_normal_yclient,decode_response_simplepir_yclient)src/modulus_switch.rsmod testrecoverandswitchThe two side-channel demonstration tests are marked
#[ignore]since they are expected to fail (they document the vulnerability rather than assert correct behavior).How to reproduce