From c5f0fd9ca6ac5a0c5fc713670b2f72fa38e40d83 Mon Sep 17 00:00:00 2001 From: Nicolas Dreno Date: Sat, 12 Sep 2026 11:36:20 +0200 Subject: [PATCH] Zero-pad the C input in the FFI harness so its over-reads are defined libinjection reads a few bytes past the end of the input on some adversarial inputs: an ASan-confirmed out-of-bounds read in htmlencode_startswith, reached from libinjection_is_xss, on a URL-attribute value ending in an incomplete HTML entity. Past the buffer the bytes are indeterminate, so the C verdict is not reproducible across builds (the differential fuzzer saw "C: true" on the CI Linux build and "C: false" locally on byte-identical input). Copy the input into a zeroed, over-allocated buffer before handing it to libinjection, for both detectors, so any such read lands on defined zero bytes. The differential then compares against a stable C answer rather than stack garbage, and the memory-safe port (which never reads past the input) matches it. A dedicated test pins the input that surfaced this. Full corpus differential stays at 0. README and CHANGELOG updated, including the further char-semantics fixes the fuzzer surfaced. --- CHANGELOG.md | 16 +++++++++++ README.md | 8 ++++-- comparison-bin/tests/differential.rs | 22 +++++++++++++++ ffi-harness/harness.c | 41 ++++++++++++++++++++++------ 4 files changed, 76 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55edce0..dd33617 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). terminator), so `'$\0T` scans `$\0` as a number and `T'$\0T#` is flagged. This retires the last known divergence class: **no known divergence from the C library remains** on SQLi or XSS. +- Further char-semantics classes the differential fuzzer surfaced, each fixed + following C: a variable name and the binary/hex/decimal number scans stop at + or consume a NUL as C's `strchr`-based helpers do; `sp_password` and the + collate `_` check search the raw token bytes rather than a lossily decoded + string; NUL counts as whitespace in the HTML5 tokenizer (`h5_is_white`), as + it does in C's `strchr(" \t\n\v\f\r", ch)`; and a variable token stores its + name without the leading `@`, so the function fold matches a name like + `@pasSword`. The SQLi detector then found no divergence over a two-hour + differential-fuzzing campaign. ### Added - `lookup_word_type`, a presence-aware keyword lookup returning `None` only @@ -40,6 +49,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed - The differential fingerprint ceiling is lowered from 1,631 to 0: SQLi fingerprints now match the C library exactly across the corpus. +- The FFI harness zero-pads the C input. libinjection reads a few bytes past + the end of the buffer on some adversarial inputs (an ASan-confirmed + out-of-bounds read in `htmlencode_startswith`, reached from + `libinjection_is_xss`), which makes its verdict depend on whatever sits past + the input. Padding makes those reads land on defined zero bytes, so the + differential compares against a deterministic C answer; the memory-safe port + never reads past the input. ## Baseline diff --git a/README.md b/README.md index 4ce9250..16333ce 100644 --- a/README.md +++ b/README.md @@ -20,9 +20,11 @@ The fixes that got here, each following the C control flow rather than a particu - multi-word keyword folding by table presence (`LOCK IN SHARE MODE`) - a case-insensitive `INTO` check in the three-token whitelist (`into outfile`) - a length-limited XSS event-handler check (`onerror%09=`) -- a `strlenspn` that counts an embedded NUL as a set member, as C's `strchr` does +- the `strchr`-over-a-literal family, where C counts an embedded NUL as a set member (its `strchr` finds the accept string's terminator): `strlenspn`/`strlencspn` in the number, money and variable scans, and NUL as whitespace in both `char_is_white` and the HTML5 `h5_is_white` +- `sp_password` and the collate `_` check searched over the raw bytes, as C does, rather than a lossily decoded string +- a variable token value stored without the leading `@`, so the function fold matches a name like `@pasSword` -This is not the same as "provably identical". Beyond the corpus and these characterised edges, [differential fuzzing](#fuzzing) still finds new divergences, including false positives, within seconds. Treat this port as a close match with a measured gap of zero on the corpus, not as a drop-in replacement whose every answer is guaranteed. +This is not the same as "provably identical", but the measured gap is small and shrinking. Over a two-hour differential-fuzzing campaign the SQLi detector found no divergence at all. The XSS detector's one finding was a bug in the C library rather than the port: an [ASan](https://clang.llvm.org/docs/AddressSanitizer.html)-confirmed out-of-bounds read in `htmlencode_startswith`, whose verdict then depends on whatever sits past the buffer. The FFI harness zero-pads the C input so that read is defined and the comparison is deterministic; the memory-safe port never reads past the input. Treat this port as a close match with a measured gap of zero on the corpus, not as a drop-in replacement whose every answer is guaranteed. ## Testing and CI @@ -31,7 +33,7 @@ CI runs on every push and pull request: - **Lints and unit tests** across the workspace. - **Library coverage** (`cargo-llvm-cov`) with an enforced floor. - **Differential against the C library** — the job that matters, described above. A divergence fails it. -- **Differential fuzzing** (two minutes per detector). It **reports rather than gates**: it still surfaces new divergence classes faster than they are fixed, so gating on it would mean either a red build forever or an exception list that excuses everything. The corpus differential is the gate; this job keeps new classes visible. +- **Differential fuzzing** (two minutes per detector on each push, and a longer scheduled campaign). It **reports rather than gates**: a probabilistic run is a weak signal to block a merge on, so the corpus differential is the gate and this job keeps any new class visible. The C side runs through the zero-padded harness, so a divergence it reports is a genuine parse difference rather than C reading past the buffer. A separate test asserts that neither detector panics on adversarial input: every one- and two-byte value including NUL, random metacharacter strings, and 50,000-byte pathological repeats. diff --git a/comparison-bin/tests/differential.rs b/comparison-bin/tests/differential.rs index 664ea28..ed6c2cf 100644 --- a/comparison-bin/tests/differential.rs +++ b/comparison-bin/tests/differential.rs @@ -441,3 +441,25 @@ fn at_variable_named_like_a_function_matches_the_c_library() { assert_eq!(c_fp, "f(f(1", "C folds the variable to a function"); assert!(c_is, "C flags this injection, and so must the port"); } + +/// Guards an input on which libinjection's C code reads past the end of the +/// buffer: an ASan-confirmed over-read in `htmlencode_startswith`, reached from +/// `libinjection_is_xss`, on a URL-attribute value ending in an incomplete HTML +/// entity. Past the buffer the bytes are indeterminate, so the C verdict is not +/// reproducible across builds (it was `true` on the CI Linux build and `false` +/// here) until the FFI harness zero-pads the input. With the padded harness the +/// C answer is deterministic and the memory-safe port matches it. The differ- +/// ential fuzzer found this because it compared against an unpadded C build. +#[test] +fn xss_over_read_input_matches_the_c_library_with_a_padded_buffer() { + let input: &[u8] = &[ + 0x22, 0x60, 0x3d, 0x27, 0x27, 0x58, 0x00, 0x54, 0x6f, 0x3d, 0x26, 0x23, 0x58, 0x00, + 0x74, 0x6f, 0x5b, 0x26, 0x23, 0x58, 0x00, 0x74, 0x6f, 0x3d, 0x26, 0x23, 0x58, 0x00, + 0x54, 0x6f, 0x3d, 0x26, 0x23, 0x58, 0x00, 0x74, 0x6f, 0x3d, 0x26, 0x23, 0x58, + ]; + assert_eq!( + libinjectionrs::detect_xss(input).is_injection(), + c_xss(input), + "XSS over-read input diverges from the C library under the padded harness" + ); +} diff --git a/ffi-harness/harness.c b/ffi-harness/harness.c index 2530e8e..8ba0ad6 100644 --- a/ffi-harness/harness.c +++ b/ffi-harness/harness.c @@ -2,15 +2,35 @@ #include "libinjection.h" #include "libinjection_sqli.h" #include "libinjection_xss.h" +#include #include +// libinjection reads a few bytes past the end of the input on some adversarial +// inputs (an ASan-confirmed over-read in htmlencode_startswith, reached from +// libinjection_is_xss). Past the buffer the bytes are indeterminate, so the +// verdict is not reproducible across builds. Copy the input into a zeroed, +// over-allocated buffer so any such read lands on defined zero bytes and the +// C result is deterministic. The differential then compares against a stable +// C answer rather than stack garbage. +#define HARNESS_PAD 16 + +static char* padded_copy(const char* input, size_t input_len) { + char* buf = (char*)calloc(input_len + HARNESS_PAD, 1); + if (buf && input_len) { + memcpy(buf, input, input_len); + } + return buf; +} + sqli_result_t harness_detect_sqli(const char* input, size_t input_len, int flags) { sqli_result_t result = {0}; struct libinjection_sqli_state state; - + + char* buf = padded_copy(input, input_len); + // Initialize state - libinjection_sqli_init(&state, input, input_len, flags); - + libinjection_sqli_init(&state, buf, input_len, flags); + // Detect SQL injection result.is_sqli = libinjection_is_sqli(&state); @@ -29,19 +49,24 @@ sqli_result_t harness_detect_sqli(const char* input, size_t input_len, int flags } else { result.fingerprint[0] = '\0'; } - + + free(buf); return result; } xss_result_t harness_detect_xss(const char* input, size_t input_len, int flags) { xss_result_t result = {0}; - + + char* buf = padded_copy(input, input_len); + // Detect XSS - result.is_xss = libinjection_xss(input, input_len); - + result.is_xss = libinjection_xss(buf, input_len); + + free(buf); + // Note: flags parameter currently unused but kept for API consistency (void)flags; - + return result; }