fix(pg-core,security): bounds-check malformed ciphertext input#150
fix(pg-core,security): bounds-check malformed ciphertext input#150dobby-coder[bot] wants to merge 3 commits intomainfrom
Conversation
Several code paths in pg-core would panic on truncated or attacker- crafted ciphertext. Replaced unchecked slice::split_at calls with a new try_split_at helper that returns FormatViolation, added explicit length checks in extract_policy and verify_segment (both Rust and WASM variants), and replaced debug_assert! on seg.len() > SIG_BYTES with runtime Err returns so release builds reject truncated segments cleanly. Added 3 unit tests covering empty input, truncated-after-preamble, and garbage input. All 34 pg-core tests pass. Closes #138
|
/dobby add some tests. |
|
Dobby is on it, sir! Dobby sees Master Ruben wants more tests for the malformed ciphertext fixes. Dobby will pull the branch, study the existing tests, and add tests covering the stream unsealer bounds checks too. Dobby will report back very soon! |
Adds unit tests for the try_split_at helper and preamble_checked, plus more Unsealer::new boundary cases on both the in-memory and stream paths: each truncation point (preamble, header, sig length, sig bytes) surfaces the expected FormatViolation/NotPostGuard instead of panicking. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Pushed a new commit with expanded test coverage (67aa349..864d1b5):
Each test asserts the specific error variant/message it expects (not just Note: I did not add direct tests for the |
|
/dobby make sure you pass the ci checks. |
|
Dobby hears Master rubenhensen! Dobby will make sure the CI checks pass on PR #150! Dobby is checking the failing checks right now and will push fixes! 🧦 |
|
CI is green. All 20 checks pass (the 4 |
Summary
Addresses the three findings in #138. Several code paths in
pg-corewould panic on truncated or attacker-crafted ciphertext. This PR:client/rust/mod.rs,client/web/mod.rs): replaced uncheckedslice::split_atwith a newutil::try_split_athelper that returnsError::FormatViolationwhenmid > b.len().extract_policy(client/rust/stream.rs,client/web/stream.rs): added explicitbuf.len() < POL_SIZE_SIZEandbuf.len() < POL_SIZE_SIZE + pol_lenchecks, pluschecked_addfor the policy-length addition so a craftedpol_lencannot overflow.debug_assert!(seg.len() > SIG_BYTES)with runtimeErrreturns, so release builds no longer wrap/underflow when a decrypted segment is too short for its trailing signature. Applied in both Rust (verify_segment) and WASM (two sites around thefinal_plain/per-segment signature checks).Tests
Three new unit tests exercise the in-memory unsealer against malformed input:
test_unseal_rejects_empty_input— empty slice must not panic.test_unseal_rejects_truncated_after_preamble— a valid preamble + 1 byte must returnFormatViolation, not panic.test_unseal_rejects_garbage_input— 1 KiB of zeros must return someErr, not panic.```
$ cargo test -p pg-core --features test,stream,rust
test result: ok. 34 passed; 0 failed; 0 ignored
```
Notes
Error::FuturesIOissue on the wasm32-unknown-unknown target unrelated to this change).util.rs::preamble_checked— it already bounds-checks correctly.Reviewer quickstart
```
git fetch origin && git checkout fix/pg-core-panic-on-malformed && cargo test -p pg-core --features test,stream,rust
```
Closes #138