Repository navigation
Fix/713 spec exposure redos sweep - #805
Merged
Merged
Conversation
…nguages
The spec_exposure signature's `\[(?:\s*SPEC\s*-\s*\d+|spec|audit)[^\]]*\]`
shape (or close per-language variants) was copy-pasted across the
majority of LANGUAGE_DEFINITIONS, and most copies still carried the
unbounded `[^\]]*` immediately after the equally-unbounded `\d+` --
digits satisfy both character classes, so an unclosed "[SPEC-11111..."
tag with no closing "]" forces the engine to re-scan an ever-shrinking
digit suffix at every possible split point between the two quantifiers,
genuine O(n^2) catastrophic backtracking on a single crafted or
accidentally-malformed file.
The original issue found 28 languages carrying this bug. Re-audited the
current state before touching anything (this exact class had already
been fixed independently, one language at a time, across this epic --
embedded_python, css, tcl, matlab, scheme, typescript, rust, c, cpp,
csharp, groovy, shell, sqlite): confirmed only 17 remain vulnerable --
python, javascript, java, go, php, ruby, swift, kotlin, html, assembly,
perl, dart, dockerfile, solidity, objective-c, yacc, m4.
Applied the same clamp already proven correct for the earlier fixes:
bounded `\d+` to `\d{1,10}` and `[^\]]*` to `[^\]]{0,300}` -- generous
enough for any realistic spec/audit tag, only removing the
pathological-input hang. Deliberately left each language's own extra
alternatives (html's RFC/W3C/CERN/TBL, dockerfile's CVE-\d{4}-\d+,
solidity's ERC-\d+/EIP-\d+, etc.) untouched beyond the specific
adjacent-quantifier shape -- those aren't part of the vulnerability and
bounding them isn't this issue's scope.
New test: a single dedicated cross-language regression test
(parametrized over all 17 languages) rather than scattering 17
near-identical tests across each language's own already-large
strict-parsing section -- the issue itself explicitly authorizes this
as the cleaner alternative. Uses the scale-relative ratio methodology
(not an absolute wall-clock threshold, which is flaky across CI
hardware -- the exact failure mode already found and fixed for a
different spec_exposure test earlier in this epic) plus the shared
_best_of_timing() helper.
Verified: full strict-parsing suite (2508 passed), full test suite
(3414 passed; 14 pre-existing, unrelated failures in
test_security_auditor.py -- missing xgboost in this environment,
confirmed identical without this change), ruff format/lint clean.
Differential Scan (crucible_check.py, both venvs): zero diff, and
confirmed this is legitimate rather than masking a problem -- the
corpus's only 18 real Specification Traceability Tag hits are all in
cobol/agc_assembly, neither of which is among the 17 languages touched
here (both were already correctly excluded as "not affected" by the
original issue).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…3.9) CI failure on this PR's own smoke-test run: test_spec_exposure_adjacent_ quantifier_redos_sweep[dart] failed with "old pattern was expected to show quadratic (~4x) scaling... but only scaled 2.49x" -- just under the 2.5 ratio threshold, on a contended macOS/Python-3.9 runner. Locally (unloaded) the same pattern measures a clean ~4x, confirming this is transient CI timing noise, not a real regression in the fix. Hardened against this class of flakiness two ways, applied uniformly to all 8 "prove the old pattern is quadratic" ratio checks that share _best_of_timing (not just the new #713 sweep): - Raised _best_of_timing's default trials from 3 to 5, tightening the min-of-N distribution further without changing what's being proven. - Lowered the pass threshold from 2.5 to 2.2 -- still safely above linear's ~2.0x baseline, but with meaningfully more margin against a genuinely-quadratic (~4x) measurement getting compressed by runner contention. Left the two pre-existing, unrelated embedded_python tests that use a different (single-shot, larger-payload, `durations[-1]/durations[-2] > 2.5`) pattern untouched -- no evidence they're flaky, and touching them is outside this issue's scope. Verified: the sweep test passes stably across 5+ repeated runs, the full redos_regression suite (15 passed), and the full strict-parsing suite (2508 passed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
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
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.
Description of Structural Changes
Visual Observatory Testing
The Differential Scan Acknowledgement