test: close the fuzz and property-test gaps a security review found

.github/workflows/build.yaml

@@ -261,6 +261,8 @@ jobs:
         run: cargo +nightly fuzz run utf16 --target x86_64-unknown-linux-gnu -- -max_total_time=30
       - name: Fuzz column_value
         run: cargo +nightly fuzz run column_value --target x86_64-unknown-linux-gnu -- -max_total_time=30
+      - name: Fuzz parse_attributes
+        run: cargo +nightly fuzz run parse_attributes --target x86_64-unknown-linux-gnu -- -max_total_time=30
 
   # Verifies the crate can actually be packaged, without publishing anything.
   #

AGENTS.md

@@ -1324,20 +1324,35 @@ let ptr = unsafe { arena.as_mut_ptr().cast::<u8>().add(1) }.cast::<u16>();
 ### Fuzzing
 
 `fuzz/` holds [cargo-fuzz](https://github.com/rust-fuzz/cargo-fuzz) targets for
-the memory-marshalling hot paths, `write_column_value` and `utf16`. Each
-allocates its output buffer at exactly the caller-declared length, so
-AddressSanitizer catches any overrun that clippy cannot see. It is its own
-Cargo workspace, because libFuzzer needs nightly, so the root build ignores it.
-A short smoke run of both targets runs on every PR (the `fuzz` job in
-`build.yaml`).
+the raw-pointer paths: `write_column_value`, `utf16` and
+`ffi::setup::parse_attributes_w`. The first two allocate their output buffer at
+exactly the caller-declared length, so AddressSanitizer catches any overrun that
+clippy cannot see; the third walks a Driver-Manager pointer looking for a
+terminator. It is its own Cargo workspace, because libFuzzer needs nightly, so
+the root build ignores it. A short smoke run of every target runs on every PR
+(the `fuzz` job in `build.yaml`).
 
 ```bash
 cargo install cargo-fuzz
 cargo +nightly fuzz run utf16
 cargo +nightly fuzz run column_value
+cargo +nightly fuzz run parse_attributes
 ```
 
-See [`fuzz/README.md`](fuzz/README.md) for what is and is not worth fuzzing.
+**A fuzz target is for `unsafe` code.** Where the code is safe Rust the worst
+outcome is a panic or a wrong answer, and a `proptest` suite next to the code
+finds both on stable, in far less CPU time, on every `cargo test` rather than in
+a 30-second smoke run. `escape`, `types::connect_params`, `param_convert`,
+`numeric_convert` and `types::conversions` are covered that way, with an oracle
+rather than only a never-panics assertion wherever one exists.
+
+Reaching a `pub(crate)` item from `fuzz/`, which is a separate crate, goes
+through a wrapper gated on the default-off `test-support` feature.
+`parse_attributes_summary_w` is the example to copy.
+
+See [`fuzz/README.md`](fuzz/README.md) for what is and is not worth fuzzing, and
+for why a fuzz target must terminate the buffer it hands to a parser whose
+contract says it is terminated.
 
 ### Benchmarks
 

CHANGELOG.md

@@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- `test_support::parse_attributes_summary_w`, behind the default-off
+  `test-support` feature. It reports what the `ConfigDSNW` attribute-list parser
+  read from a raw `*const u16`, so the new `parse_attributes` fuzz target can
+  reach a `pub(crate)` parser from a separate crate. A driver has no reason to
+  call it, and a build without the feature does not export it.
+- A `parse_attributes` fuzz target, covering that parser over aligned,
+  deliberately misaligned and overlong-segment buffers. It joins the per-PR
+  ASAN smoke run.
+- Property tests for the paths that carry no `unsafe` and so are not worth a
+  fuzz target: the escape translator against a grammar of escape tokens and four
+  dialects, `ConnectParams` round-tripping a whole connection string rather than
+  one pair, the two parameter-conversion tables, and every `*_from_raw` swept
+  across its entire input domain.
+
 ## [0.1.0] — 2026-08-04
 
 First release, so this section describes what the crate offers rather than what

Cargo.toml

@@ -39,6 +39,9 @@ exclude = [
     "rustfmt.toml",
     ".pre-commit-config.yaml",
     ".markdownlint.yaml",
+    # Saved `proptest` failure seeds. Test-only data, and the tests that read it
+    # are not compiled from the published tarball.
+    "proptest-regressions/",
 ]
 
 # docs.rs builds for a single target by default, which would leave `ConfigDSNW`
@@ -52,14 +55,18 @@ targets = ["x86_64-pc-windows-msvc", "x86_64-unknown-linux-gnu"]
 features = ["test-support"]
 
 [features]
-# Test support for driver crates: the `conformance` module, which drives
-# `SQLGetInfoW` through the real C ABI to check an info type's return shape.
+# Test support for driver crates and for `fuzz/`. Two things:
+#
+# - the `conformance` module, which drives `SQLGetInfoW` through the real C ABI
+#   to check an info type's return shape;
+# - `test_support::parse_attributes_summary_w`, which lets the `parse_attributes`
+#   fuzz target reach a `pub(crate)` raw-pointer parser from a separate crate.
 #
 # Default-off because it is test code. Compiled unconditionally it lands in
 # every driver's production binary, and it reaches an `unreachable!()` through a
 # public `unsafe fn` taking a caller-supplied `u16` — a panic path a shipped
 # driver has no reason to carry. Driver test suites enable it under
-# `[dev-dependencies]`.
+# `[dev-dependencies]`; `fuzz/Cargo.toml` enables it on its path dependency.
 test-support = []
 
 [dependencies]

fuzz/Cargo.toml

@@ -25,7 +25,16 @@ test = false
 doc = false
 bench = false
 
+[[bin]]
+name = "parse_attributes"
+path = "fuzz_targets/parse_attributes.rs"
+test = false
+doc = false
+bench = false
+
 [dependencies]
 arbitrary = { version = "1", features = ["derive"] }
 libfuzzer-sys = "0.4"
-stackable-odbc-core = { path = ".." }
+# `test-support` is what makes `ffi::setup::parse_attributes_summary_w` visible.
+# The attribute parser itself is `pub(crate)`, and this is a separate crate.
+stackable-odbc-core = { path = "..", features = ["test-support"] }

fuzz/README.md

@@ -1,25 +1,58 @@
 # Fuzz targets
 
-These targets cover the `unsafe` pointer-marshalling paths in
-`stackable-odbc-core`, which is where AddressSanitizer catches what clippy
-cannot see. Each one allocates its output buffer at exactly the size a correct
-application would, so any write past the end is a reported error rather than a
-silent overrun into neighbouring memory.
+These targets cover the `unsafe` raw-pointer paths in `stackable-odbc-core`,
+which is where AddressSanitizer catches what clippy cannot see.
 
-That size is the `BufferLength` argument for a variable-length C target, and the
-C type's own size for a fixed-length one, which ignores `BufferLength`
-altogether.
+The two marshalling targets allocate their output buffer at exactly the size a
+correct application would, so any write past the end is a reported error rather
+than a silent overrun into neighbouring memory. That size is the `BufferLength`
+argument for a variable-length C target, and the C type's own size for a
+fixed-length one, which ignores `BufferLength` altogether.
+
+The third goes the other way: it hands a parser an *input* buffer sized exactly
+to its contents, so a read that runs past the terminator is reported rather than
+finding more of the same allocation.
 
 - `utf16` covers `utf16_to_string` and `write_utf16`.
 - `column_value` covers `write_column_value` across every marshallable value
   variant and C target type, which is the full coercion matrix.
+- `parse_attributes` covers `ffi::setup::parse_attributes_w`, the attribute-list
+  walk behind `ConfigDSNW`, over aligned, deliberately misaligned and
+  overlong-segment buffers.
+
+## What belongs here, and what does not
+
+The line is `unsafe`, not importance. A target earns its nightly toolchain and
+its ASAN build when the failure it is hunting is a read or write outside an
+allocation. Where the code is safe Rust, the worst outcome is a panic or a wrong
+answer, and a property test finds both on stable, in a fraction of the CPU time,
+on every PR rather than in a 30-second smoke run.
+
+So the pure-safe parsers are covered by [`proptest`](https://docs.rs/proptest)
+suites next to the code instead, asserting never-panics *and* an oracle wherever
+one exists:
 
-The pure-safe parsers, `translate_escapes`, `ConnectParams::parse` and the
-drivers' own type-name parsers, contain no `unsafe`, so AddressSanitizer adds
-nothing over property tests. They are covered by
-[`proptest`](https://docs.rs/proptest) suites next to the code, which run on
-stable under an ordinary `cargo test` and assert both never-panics and
-round-trip invariants.
+| Module | What the properties assert |
+| --- | --- |
+| `escape` | A grammar of escape-ish tokens against four dialects; an unknown escape and its surroundings survive byte for byte |
+| `types::connect_params` | Every pair survives `parse` ∘ `to_connection_string`, and no keyword is injected by a value containing `}` |
+| `param_convert` | Rendering never expands past `MAX_DECIMAL_EXPANSION_DIGITS`; rendering round-trips; `to_integer` agrees with `i128::from_str`; a `SQL_NUMERIC_STRUCT` reconstructs its literal |
+| `numeric_convert` | The whole *C to SQL: Numeric* table is total; integers reach their target exactly when they are in range |
+| `types::conversions` | Every `*_from_raw` swept across its entire 16-bit domain, round-tripping |
+
+`parse_attributes` is on this side of the line because it steps a raw `*const
+u16` looking for a terminator, and because the Driver Manager gives it no
+alignment guarantee.
+
+### Terminate the buffer
+
+The parser's safety contract is that the pointer is null or double-null
+terminated. A fuzz target that hands it an unterminated buffer will get an ASAN
+report, and the report will be about the target: the read past the end is the
+caller breaking a contract, not the parser exceeding one. `parse_attributes`
+appends the terminator itself and fuzzes what comes before it, and reaches the
+per-segment scan limit with a run that is long but still inside a real
+allocation.
 
 ## Running
 
@@ -30,8 +63,14 @@ libFuzzer does.
 cargo install cargo-fuzz
 cargo +nightly fuzz run utf16
 cargo +nightly fuzz run column_value
+cargo +nightly fuzz run parse_attributes
 ```
 
+`parse_attributes` reaches `parse_attributes_w`, which is `pub(crate)`, through
+`test_support::parse_attributes_summary_w`. That wrapper is gated behind the
+default-off `test-support` feature, which this crate enables on its dependency,
+so a shipped driver never exports it.
+
 If cargo-fuzz fails with "sanitizer is incompatible with statically linked
 libc", it picked a musl target. Pin the gnu triple explicitly, which is what CI
 does:

fuzz/fuzz_targets/parse_attributes.rs

@@ -0,0 +1,120 @@
+//! `ConfigDSNW`'s attribute-list parser, driven over raw pointers.
+//!
+//! `test_support::parse_attributes_summary_w` wraps
+//! `ffi::setup::parse_attributes_w`, which walks a `*const u16` the Driver
+//! Manager supplied, hunting for the double null that ends the list. It is the
+//! one parser in core whose failure mode is a read past the end of an
+//! allocation rather than a panic, which is what makes AddressSanitizer worth
+//! the nightly toolchain here.
+//!
+//! # Every buffer is terminated
+//!
+//! The parser's safety contract is that the pointer is null, or points to a
+//! valid double-null-terminated `u16` sequence. So each shape below appends that
+//! terminator itself and fuzzes what comes *before* it. Handing the parser an
+//! unterminated buffer would certainly produce an ASAN report, and it would be a
+//! report about this file: the read past the end would be the caller breaking a
+//! contract it agreed to, not the parser exceeding one. What is worth fuzzing is
+//! the walk over a buffer that is terminated but says nothing else sensible.
+//!
+//! The parser bounds each segment at `i16::MAX` code units precisely so a caller
+//! that gets this wrong is contained rather than unbounded, and the
+//! `OverlongSegment` shape reaches that bound from inside a real allocation.
+
+#![no_main]
+
+use arbitrary::Arbitrary;
+use libfuzzer_sys::fuzz_target;
+use stackable_odbc_core::test_support::parse_attributes_summary_w;
+
+/// One code unit past the parser's own per-segment scan limit, which is
+/// `i16::MAX`. Declared here rather than imported because it is crate-private,
+/// and a copy that drifts makes this shape stop reaching the bound rather than
+/// start failing, so the assertion below checks the bound was actually hit.
+const PAST_SEGMENT_SCAN_LIMIT: usize = i16::MAX as usize + 1;
+
+#[derive(Arbitrary, Debug)]
+enum Shape {
+    /// A `u16`-aligned buffer, which is the ordinary case.
+    Aligned,
+    /// A buffer whose `u16` sequence starts at an odd byte address.
+    ///
+    /// The Driver Manager promises no alignment, and the parser reads every code
+    /// unit with `read_unaligned` for that reason. An aligned read of this
+    /// pointer is undefined behaviour, and in a debug build it aborts without
+    /// unwinding, which no panic hook can contain. A regression to
+    /// `slice::from_raw_parts` would be caught here and nowhere else.
+    Unaligned,
+    /// A segment longer than the parser will scan, so the scan limit fires with
+    /// every read still inside the allocation.
+    OverlongSegment,
+}
+
+#[derive(Arbitrary, Debug)]
+struct Input {
+    shape: Shape,
+    units: Vec<u16>,
+}
+
+fuzz_target!(|input: Input| {
+    match input.shape {
+        Shape::Aligned => {
+            let mut buf = input.units;
+            buf.extend_from_slice(&[0, 0]);
+            // SAFETY: `buf` is non-empty and ends in two zero `u16`s, so it is a
+            // double-null-terminated sequence, and it outlives the call.
+            let _ = unsafe { parse_attributes_summary_w(buf.as_ptr()) };
+        }
+
+        Shape::Unaligned => {
+            // One byte of padding in front, so the `u16` sequence begins at an
+            // odd address. Everything after it stays a whole number of code
+            // units, which keeps the terminator two aligned-to-the-sequence
+            // zeros rather than a split pair.
+            let mut bytes = vec![0u8];
+            for unit in &input.units {
+                bytes.extend_from_slice(&unit.to_le_bytes());
+            }
+            bytes.extend_from_slice(&[0, 0, 0, 0]);
+
+            // SAFETY: offset 1 is inside `bytes`, which holds `1 + 2n + 4`
+            // bytes, so the sequence from there is `n + 2` whole `u16`s ending
+            // in two zeros. The pointer is read only with `read_unaligned`, so
+            // the odd address is sound, and `bytes` outlives the call.
+            let ptr = unsafe { bytes.as_ptr().add(1) }.cast::<u16>();
+            let _ = unsafe { parse_attributes_summary_w(ptr) };
+        }
+
+        Shape::OverlongSegment => {
+            // The fuzzed units come first, so their bytes still drive real
+            // segments, and the overlong run is appended behind a separator.
+            let mut buf = input.units;
+            buf.push(0);
+
+            // Decided on the prefix alone, before the run and the terminator are
+            // appended: those end the list by construction, so a check made
+            // after them would be true every time and assert nothing.
+            let ends_early = ends_the_list(&buf);
+
+            buf.resize(buf.len() + PAST_SEGMENT_SCAN_LIMIT, u16::from(b'A'));
+            buf.extend_from_slice(&[0, 0]);
+
+            // SAFETY: as the aligned case; `buf` ends in two zero `u16`s.
+            let (_, _, syntax_error) = unsafe { parse_attributes_summary_w(buf.as_ptr()) };
+
+            // Where the run is reached at all, the scan limit must have fired.
+            assert!(
+                syntax_error || ends_early,
+                "a segment of {PAST_SEGMENT_SCAN_LIMIT} code units must trip the scan limit"
+            );
+        }
+    }
+});
+
+/// Whether the parser stops inside `units` rather than walking off its end.
+///
+/// It stops at an empty segment, which is a leading null or two consecutive
+/// ones.
+fn ends_the_list(units: &[u16]) -> bool {
+    units.first() == Some(&0) || units.windows(2).any(|pair| pair == [0, 0])
+}

proptest-regressions/param_convert.txt

@@ -0,0 +1,10 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc b397f155c0024183da7ee227f989dce4f106122c05ddcd6d0a0fd6ea193de960 # shrinks to text = "0e-1048577"
+cc 14cac6f9937a37466197f0f46da27ef8f7bf8f4e4dcd139f7ff5b9985a5b79eb # shrinks to text = "0e-2147483647"
+cc a63740d711d952f1a19c605c2d34fd42607826298239bcf9051d443bead7282b # shrinks to text = "0e-1048577"
+cc c1710352100792598c4ba42876c7f85a26ded650c3c2b3acd0d4a4dcd3163a54 # shrinks to text = ".214748364721474836472147483647", wide = 2, narrow = 3

src/backend.rs

@@ -3074,6 +3074,10 @@ mod tests {
     /// a test naming the info type rather than surviving review, which is how
     /// `SQL_GROUP_BY`, `SQL_CORRELATION_NAME` and `SQL_SUBQUERIES` come to be
     /// decided in the wrong place.
+    #[cfg_attr(
+        miri,
+        ignore = "two full SQLGetInfo sweeps; no unsafe in this module for Miri to check"
+    )]
     #[test]
     fn default_get_info_answers_are_backend_derived_or_declared_core_facts() {
         use crate::test_utils::MockAltBackend;