fix: bound what a parse rejection echoes, and test the parsers against hostile input - #10
Merged
Merged
Conversation
…t hostile input
SECURITY.md lists "denial of service through parsing" as in scope and
nothing exercised it. Probing the parsers with megabyte-scale malformed
literals showed them linear — 1 MB of hostile input is accepted or
rejected in single-digit milliseconds, 200,000-element sets in about a
hundred — and TryParse never throws. It also showed the one real weakness:
a rejection embedded the entire input in its message, and a bad bound or
element chained the BCL's exception, whose message embeds it again. A
megabyte in became a megabyte of exception message out, copied into every
log sink and, in development, returned to the client — memory and log
volume disproportionate to the request, which is exactly the line
SECURITY.md draws.
LiteralExcerpt bounds it: messages carry a 64-character excerpt plus the
input's length. Element failures in RangeFormat, SetFormat and the JSON
element converter re-throw with the inner parser's reason as an excerpt
instead of chaining it — a validated wrapper's own message survives,
bounded, without the BCL's echo coming along. Exception types are
unchanged (FormatException, OverflowException, JsonException); TryParse
still never throws. Documented under [Unreleased] in the root and core
changelogs, since InnerException and the full message text are no longer
what they were.
ParserResilienceTests (26 cases) is the suite that now backs the SECURITY.md
line: hostile range, multirange and array literals and JSON payloads are
accepted or rejected within a [Timeout] with only the documented exception
types, TryParse never throws, and no rejection message — including inner
exceptions — exceeds 512 characters. Sizes are chosen so a quadratic parser
takes minutes where a linear one takes milliseconds.
Verified (verify-the-guard): with the source fix stashed the echo test
fails ("FormatException message is 1,000,049 characters long"); a seeded
quadratic SplitSetLiterals times out the two 200k-range cases; removing
TryParse's catch fails seven cases; all pass restored. Whole suite green,
integration included.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…an error under TreatWarningsAsErrors) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ix/parser-hostile-input
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
SECURITY.md lists "denial of service through parsing" as in scope, and nothing exercised it. Probing the parsers with megabyte-scale malformed literals showed them linear — 1 MB of hostile input accepted or rejected in single-digit ms, 200,000-element sets in ~100 ms — and
TryParsenever throws. It also showed the one real weakness: a rejection embedded the entire input in its message, and a bad bound or element chained the BCL's exception, whose message embeds it again. A megabyte in became a megabyte of exception message out — copied into every log sink and, in development, returned to the client. Memory and log volume disproportionate to the request is exactly the line SECURITY.md draws.What
Internals/LiteralExcerpt.cs: messages carry a 64-character excerpt plus the input's length. Applied at every rejection site inRangeFormat,SetFormat,RangeJsonConverter,ValueSetElementJson.ParseBound,ParseElementValue, JSON element) re-throw with the inner parser's reason as an excerpt instead of chaining it — a validated wrapper's own message survives, bounded, without the BCL's echo. Exception types unchanged (FormatException,OverflowException,JsonException);TryParsestill never throws.ParserResilienceTests(26 cases): hostile range / multirange / array literals and JSON payloads — million-char digit runs, bracket floods, unterminated quotes, escape floods, 200k elements — accepted or rejected within a[Timeout]and only with the documented exception types;TryParsenever throws; no rejection message (inner exceptions included) exceeds 512 chars. Sizes chosen so a quadratic parser takes minutes where a linear one takes milliseconds.[Unreleased]entry in the root and core changelogs (the## [x.y.z]conventions ignore it; rename on release): the message shape andInnerExceptionare user-visible.docs/testing.mdrow.Verified (verify-the-guard)
range: unbalanced: FormatException message is 1,000,049 characters long.SplitSetLiterals(copy the whole inner per element): the two 200k-range cases time out.RangeFormat.TryParsewithout its catch: seven cases fail.🤖 Generated with Claude Code