Skip to content

Commit 8b038a0

Browse files
committed
Address Tokio writer pentest follow-up
1 parent d85a1fd commit 8b038a0

6 files changed

Lines changed: 144 additions & 6 deletions

File tree

crates/base64-ng-tokio/src/decoder_writer.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ const DECODE_OUTPUT_CAP: usize = 1024;
2121
/// already have been written before a later malformed quantum is observed. For
2222
/// atomic or secret-bearing frames, collect a bounded frame and use a
2323
/// non-streaming strict or `ct` decode path.
24+
///
25+
/// I/O errors from the wrapped writer during drain do not set [`Self::is_failed`];
26+
/// only internal protocol or capacity violations latch a permanent failure.
2427
pub struct DecoderWriter<W, A, const PAD: bool>
2528
where
2629
A: Alphabet,

crates/base64-ng-tokio/src/encoder_writer.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ const ENCODE_OUTPUT_CAP: usize = 1024;
2323
/// Internal cleanup is best-effort and limited to this adapter's fixed pending
2424
/// and output buffers. It cannot clear copies held by the wrapped writer, the
2525
/// caller's buffers, registers, caches, swap, or crash dumps.
26+
///
27+
/// I/O errors from the wrapped writer during drain do not set [`Self::is_failed`];
28+
/// only internal protocol or capacity violations latch a permanent failure.
2629
pub struct EncoderWriter<W, A, const PAD: bool>
2730
where
2831
A: Alphabet,

crates/base64-ng-tokio/tests/tokio_writer.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,3 +222,71 @@ async fn streaming_encoder_writer_propagates_inner_write_error() {
222222
assert_eq!(error.kind(), std::io::ErrorKind::BrokenPipe);
223223
assert!(!writer.is_failed());
224224
}
225+
226+
#[tokio::test]
227+
async fn streaming_encoder_writer_round_trips_large_input_with_one_byte_backpressure() {
228+
let input: Vec<u8> = (0u8..=250).cycle().take(5000).collect();
229+
let expected = STANDARD.encode_vec(&input).unwrap();
230+
let inner = ScriptedWriter::new((0..expected.len()).map(|_| WriteAction::Accept(1)));
231+
let mut writer = EncoderWriter::new(inner, STANDARD);
232+
233+
writer.write_all(&input).await.unwrap();
234+
writer.shutdown().await.unwrap();
235+
236+
assert_eq!(writer.into_inner().output(), expected);
237+
}
238+
239+
#[tokio::test]
240+
async fn streaming_decoder_writer_round_trips_large_input_with_one_byte_backpressure() {
241+
let input: Vec<u8> = (0u8..=250).cycle().take(5000).collect();
242+
let encoded = STANDARD.encode_vec(&input).unwrap();
243+
let inner = ScriptedWriter::new((0..input.len()).map(|_| WriteAction::Accept(1)));
244+
let mut writer = DecoderWriter::new(inner, STANDARD);
245+
246+
writer.write_all(&encoded).await.unwrap();
247+
writer.shutdown().await.unwrap();
248+
249+
assert_eq!(writer.into_inner().output(), input);
250+
}
251+
252+
#[tokio::test]
253+
async fn streaming_encoder_writer_clamps_single_large_poll_write_to_queue_capacity() {
254+
let input: Vec<u8> = (0u8..=250).cycle().take(2000).collect();
255+
let expected = STANDARD.encode_vec(&input).unwrap();
256+
let mut writer = EncoderWriter::new(Vec::new(), STANDARD);
257+
let waker = noop_waker();
258+
let mut context = Context::from_waker(&waker);
259+
260+
let accepted = Pin::new(&mut writer)
261+
.poll_write(&mut context, &input)
262+
.map(|result| result.unwrap());
263+
264+
assert_eq!(accepted, Poll::Ready(768));
265+
assert_eq!(writer.buffered_output_len(), 1024);
266+
267+
writer.write_all(&input[768..]).await.unwrap();
268+
writer.shutdown().await.unwrap();
269+
270+
assert_eq!(writer.into_inner(), expected);
271+
}
272+
273+
#[tokio::test]
274+
async fn streaming_decoder_writer_clamps_single_large_poll_write_to_queue_capacity() {
275+
let input: Vec<u8> = (0u8..=250).cycle().take(2000).collect();
276+
let encoded = STANDARD.encode_vec(&input).unwrap();
277+
let mut writer = DecoderWriter::new(Vec::new(), STANDARD);
278+
let waker = noop_waker();
279+
let mut context = Context::from_waker(&waker);
280+
281+
let accepted = Pin::new(&mut writer)
282+
.poll_write(&mut context, &encoded)
283+
.map(|result| result.unwrap());
284+
285+
assert_eq!(accepted, Poll::Ready(1364));
286+
assert_eq!(writer.buffered_output_len(), 1023);
287+
288+
writer.write_all(&encoded[1364..]).await.unwrap();
289+
writer.shutdown().await.unwrap();
290+
291+
assert_eq!(writer.into_inner(), input);
292+
}

docs/PANIC_POLICY.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,10 @@ The current reviewed exceptions are:
5757
length/allocation invariant break. Do not use them for untrusted length
5858
metadata, constrained allocation environments, or code paths that require
5959
recoverable errors; use the fallible encode helpers instead.
60-
- Stream `get_ref`, `get_mut`, and `into_inner` internal helpers use
61-
`unreachable!` if a wrapper has already consumed its inner value. The public
62-
API consumes `self` for `into_inner`/`finish`, so this state is not reachable
63-
through safe public calls.
60+
- Core stream and Tokio writer `get_ref`, `get_mut`, and `into_inner` internal
61+
helpers use `unreachable!` if a wrapper has already consumed its inner value.
62+
The public API consumes `self` for `into_inner`/`finish`, so this state is not
63+
reachable through safe public calls.
6464

6565
Test code, doctest examples, and Kani proof harnesses may use `unwrap`,
6666
`expect`, or panic-like macros when they are asserting expected outcomes.

scripts/validate-panic-policy.sh

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ check_file() {
4545
if ($0 ~ /unreachable!\("stream .* was already taken"\)/) {
4646
allowed = 1
4747
}
48+
if ($0 ~ /unreachable!\("tokio .* writer inner writer was already taken"\)/) {
49+
allowed = 1
50+
}
4851
if ($0 ~ /unreachable!\("base64 encoder produced non-UTF-8 output"\)/) {
4952
allowed = 1
5053
}
@@ -79,9 +82,9 @@ check_file() {
7982

8083
test -s docs/PANIC_POLICY.md
8184

82-
find src -name '*.rs' | sort | while IFS= read -r source_file; do
85+
find src crates/*/src -name '*.rs' | sort | while IFS= read -r source_file; do
8386
case "$source_file" in
84-
src/encode_surface_tests.rs|src/kani_proofs.rs|src/tests.rs|src/simd/tests.rs|src/simd/wasm.rs|src/simd/neon_decode_tests.rs|src/simd/x86_decode_tests.rs)
87+
src/encode_surface_tests.rs|src/kani_proofs.rs|src/tests.rs|src/simd/tests.rs|src/simd/wasm.rs|src/simd/neon_decode_tests.rs|src/simd/x86_decode_tests.rs|crates/*/src/tests.rs|crates/*/src/*_tests.rs)
8588
continue
8689
;;
8790
esac

security/pentest/v1.3.1.md

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# base64-ng v1.3.1 Pentest Report
2+
3+
Status: PASS WITH FOLLOW-UP FIXES
4+
5+
Reviewed-Commit: d85a1fd Add Tokio async writer adapters
6+
Tester: Maintainer-supplied external pentest and CI review
7+
Scope: v1.3.0 through the Tokio async writer adapter checkpoint
8+
Date: 2026-07-03
9+
Report-Source: supplied `PENTEST.md` review for the `v1.3.0..HEAD`
10+
async writer adapter diff.
11+
12+
## Scope
13+
14+
Reviewed the new `base64-ng-tokio` writer adapter surface:
15+
16+
- `crates/base64-ng-tokio/src/encoder_writer.rs`
17+
- `crates/base64-ng-tokio/src/decoder_writer.rs`
18+
- `crates/base64-ng-tokio/src/queue.rs`
19+
- public re-export and async documentation updates
20+
21+
The review found no Critical, High, or exploitable memory-safety bugs. The
22+
reviewer noted that the Tokio companion crate denies unsafe code and that the
23+
new state machines use fixed internal buffers with explicit cleanup.
24+
25+
## Findings
26+
27+
The supplied report identified four follow-up items:
28+
29+
1. Add a permanent `security/pentest/v1.3.1.md` artifact before tagging, so the
30+
async writer admission has the same on-disk review trail as previous
31+
releases.
32+
2. Extend panic-policy validation to companion crates. The old script scanned
33+
only `src/`, so reviewed panic-like sites in `crates/*/src` were not covered
34+
by the control.
35+
3. Add committed tests for large-input capacity-clamp and one-byte-backpressure
36+
behavior in `EncoderWriter` and `DecoderWriter`.
37+
4. Document that wrapped-writer I/O errors during drain do not latch
38+
`is_failed`; only internal protocol or capacity violations permanently fail
39+
the adapter.
40+
41+
## Remediation
42+
43+
The follow-up commit after the review:
44+
45+
- added this permanent pentest artifact,
46+
- updated `scripts/validate-panic-policy.sh` to scan `src` and `crates/*/src`,
47+
- documented the reviewed Tokio writer `unreachable!` pattern in
48+
`docs/PANIC_POLICY.md`,
49+
- added large/backpressure Tokio writer regression tests, and
50+
- documented the retryable wrapped-writer I/O error behavior in both writer
51+
adapter doc comments.
52+
53+
## Verification
54+
55+
Required verification for the remediation commit:
56+
57+
- `cargo test -p base64-ng-tokio --all-features`
58+
- `cargo clippy -p base64-ng-tokio --all-targets --all-features -- -D warnings`
59+
- `scripts/checks.sh`
60+
61+
The temporary `PENTEST.md` input is not retained in the repository.

0 commit comments

Comments
 (0)