From 9b3233dd0d6fac6a29e7d9f1a63f8e156806c675 Mon Sep 17 00:00:00 2001 From: Jim Manico Date: Thu, 24 Sep 2026 19:50:30 -0700 Subject: [PATCH] Make the denial-of-service invariant promise what can be kept Invariant 8 promised that sanitize() never lets an OutOfMemoryError out "for input of any size". No library can keep that: memory in proportion to the input is unavoidable, the input string included, so an input too big for the heap can always exhaust it. The review of #510 used that wording to flag cases no input can reach. It also called output that "grows without bound" a bug, when all output grows with its input. Say what is meant: stack bounded by a constant, time and memory linear in the input, output larger by at most a constant factor, and bounding input size is the caller's job. Also size hostile-size tests for a small heap. #510 briefly carried a churn test needing a gigabyte, which passed CI on large runners and failed on smaller machines, since surefire sets no heap. The whole suite passes at -Xmx256m; say to check that. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e645d517..22ca0a74 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,12 +37,12 @@ Hard rules. Never violate them, never refactor around them. 5. **Encoding and rendering are part of the security boundary.** Changes to `Encoding`, `HtmlStreamRenderer`, `TagBalancingHtmlStreamEventReceiver`, `HtmlTextEscapingMode`, entity handling, or serialization are security changes and require adversarial tests. 6. **The prepackaged policies in `Sanitizers` are published safety promises.** Never widen what they allow. 7. **Round-trip safety:** sanitized output, re-parsed by a browser, must not mutate into a different, unsafe DOM (mXSS). -8. **Any input terminates, in bounded stack, memory and time.** `sanitize()` must never throw, and never let a `StackOverflowError` or `OutOfMemoryError` out, for input of any size or shape. Recursion whose depth follows the input is forbidden: bound it with a small constant and drop what exceeds the bound, as `CssGrammar.MAX_FUNCTION_DEPTH` does, or walk with an explicit stack. Work that grows faster than linearly with the input, and output that grows without bound, are bugs of the same class; the tag balancer's quadratic paths and its formatting reconstruction were both found the hard way. A crash, a hang, or a blow-up on hostile input is a denial of service and is handled as a vulnerability. +8. **Any input terminates, in bounded stack and in time and memory linear in its size.** `sanitize()` must never throw, and never let a `StackOverflowError` out, for input of any shape. Recursion whose depth follows the input is forbidden: bound it with a small constant and drop what exceeds the bound, as `CssGrammar.MAX_FUNCTION_DEPTH` does, or walk with an explicit stack. Time and memory may grow only linearly with the input, and output only by a constant factor; the tag balancer's quadratic paths, and its formatting reconstruction that turned 70 KB into 17 MB, were both found the hard way. Memory in proportion to the input is unavoidable, so an input too big for the heap can still exhaust it; bounding input size is the caller's job, and that alone is not a bug. A crash, a hang, or a blow-up on hostile input is a denial of service and is handled as a vulnerability. ## Change Rules - Small, single-purpose changes only. No drive-by refactors of parser, policy, or renderer code. -- Any change touching parsing, policy enforcement, rendering, encoding, URL handling, or CSS handling requires new regression tests with hostile payloads, not just benign inputs. Where the code walks the input recursively, keeps state per token, or re-scans what it has already seen, one of those tests runs at hostile size, hundreds of thousands of nested or repeated tokens, and shows no error and linear time. +- Any change touching parsing, policy enforcement, rendering, encoding, URL handling, or CSS handling requires new regression tests with hostile payloads, not just benign inputs. Where the code walks the input recursively, keeps state per token, or re-scans what it has already seen, one of those tests runs at hostile size, hundreds of thousands of nested or repeated tokens, and shows no error and linear time. Size it to pass with the test JVM at `-Xmx256m` (`./mvnw test -DargLine=-Xmx256m`): surefire sets no heap, so a test needing a gigabyte passes CI and fails on smaller machines. - Never weaken, delete, or loosen an existing test to make a change pass. A failing security test means the change is wrong. - No new dependencies. The published library has zero runtime dependencies. The only compile-time dependency is `spotbugs-annotations` (`provided` scope), which brings in JSR 305 (`com.google.code.findbugs:jsr305`) for the `javax.annotation` nullability and concurrency annotations. Both are annotations only and are not shipped. Test-scope dependencies (JUnit, commons-codec, validator.nu htmlparser) must stay test-scope. Keep it that way. - Follow the Contributing section of `README.md`: open an issue first to reach the maintainers, and include both positive and negative tests in any PR that changes behavior or adds functionality.