Skip to content

🐛 fix(relaxng): reject invalid grammars at compile - #951

Merged
gaborbernat merged 1 commit into
tox-dev:mainfrom
gaborbernat:fix/relaxng-compile-guards
Oct 1, 2026
Merged

gaborbernat merged 1 commit into
tox-dev:mainfrom
gaborbernat:fix/relaxng-compile-guards

Conversation

@gaborbernat

@gaborbernat gaborbernat commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Compiling a RelaxNG schema accepted several grammar classes the specification forbids and the C derivative engine cannot process safely, so a malformed or adversarial schema crashed, hung, or exhausted memory (CWE-476, CWE-674, CWE-407). 🔒 The fix enforces the restrictions at compile, so an invalid grammar fails fast with a ValueError instead of at validation time; the thin Python shim is unchanged and all of the work is in the relaxng.h C core.

A <ref> with no name attribute dereferenced a NULL attribute pointer and crashed the interpreter (SIGSEGV). It is now rejected during compilation, matching libxml2's XML_RNGP_REF_NO_NAME, and the specification requires the name on ref (section 4.10).

A reference cycle whose expansion never passes through an element, a self-referential or left-recursive define, hung or overflowed the stack. Compilation now detects it with a reachable-only ref-graph walk that stamps depth across each element boundary, the section 4.19 rule that libxml2, James Clark's jing and Sun's MSV all enforce, and raises define '<name>' references itself with no element in between. Because the compile check now guarantees cycle-freedom, the now-unreachable runtime building guards in the derivative functions were removed, so the compile pass is the single source of that invariant, as it is in jing and MSV, which carry no runtime cycle guard.

An interleave whose branches can match an element with the same name, or can both match text, violates the section 7.4 restriction. It previously compiled and then drove the derivative into exponential memory on a tiny document; it is now rejected at compile, as libxml2 ("Element or text conflicts in interleave"), jing and MSV do.

A legal but ambiguous choice, for example oneOrMore(choice(a, group(a, a))), doubled the residual pattern on each child element and exhausted memory at about 26 children. The per-validation derivative now hash-conses patterns and drops a duplicate choice branch, following James Clark's derivative algorithm and the same interning jing uses in its PatternInterner/makeChoice and MSV in its ExpressionPool, so 400 children validate in flat memory, the result lxml also returns.

from turbohtml.validate import RelaxNG
from turbohtml import parse_xml

R = "http://relaxng.org/ns/structure/1.0"
RelaxNG(f'<grammar xmlns="{R}"><start><ref/></start><define name="x"><empty/></define></grammar>')
# before: SIGSEGV; after: ValueError at compile

RelaxNG(f'<grammar xmlns="{R}"><start><ref name="a"/></start><define name="a"><ref name="a"/></define></grammar>')
# before: hang / stack overflow; after: ValueError at compile

One intentional divergence from lxml: libxml2 rejects a conflicting construct even inside an unreferenced define, while this fix scans only patterns reachable from start, because an unreachable <define> is never built or validated and so cannot crash or run away. That choice is memory-safe, preserves every correctness fix for each reachable pattern, and keeps compilation of a grammar with many dead defines close to its former cost.

Library <ref> no name ref cycle with no element overlapping/ambiguous interleave/choice
turbohtml ≤ 1.13.1 SIGSEGV hang / SIGSEGV accepts, exponential memory
libxml2 2.14.6 parse error (XML_RNGP_REF_NO_NAME) "Detected a cycle in … references" "Element or text conflicts in interleave"; choice via state dedup
jing error "bad recursive reference to pattern" rejects conflict; choice linear via interning
MSV error "infinite recursion" rejects conflict; choice linear via ExpressionPool
RELAX NG spec 4.10 requires name 4.19 forbids the loop 7.4 forbids interleave competition

Any application that compiles RELAX NG schemas it does not fully control, or validates untrusted documents against an ambiguous schema, was open to a crash, hang, or memory exhaustion; there is no confidentiality or integrity impact. Schemas the validator already accepted compile and validate as before; the only accepted-to-rejected change is for the malformed schemas this closes (a nameless reference, a reference cycle, an ambiguous choice or interleave), which the RELAX NG spec forbids and other validators reject at compile.

This fixes the RELAX NG compile-time crash, cycle hang, and ambiguous-pattern memory exhaustion, tracked privately in GHSA-q28g-vj28-89fm.

@gaborbernat gaborbernat added the bug Something isn't working label Oct 1, 2026
@gaborbernat
gaborbernat force-pushed the fix/relaxng-compile-guards branch from 5c55a42 to 47691f9 Compare October 1, 2026 14:38
@codspeed

codspeed Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 5 improved benchmarks
❌ 1 regressed benchmark
✅ 574 untouched benchmarks
⏩ 32 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ test_feature[compile-rng] 89.8 µs 101.5 µs -11.49%
⚡ test_feature[validate-rng] 8 ms 6.6 ms +21.84%
⚡ test_feature[is-valid-rng-valid] 8 ms 6.6 ms +21.23%
⚡ test_feature[validate-rng-reuse-interleave] 1.4 ms 1.2 ms +19.96%
⚡ test_feature[validate-rng-reuse] 5.2 ms 4.7 ms +10.98%
⚡ test_feature[select-relative-sibling] 45.6 µs 42.9 µs +6.31%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing gaborbernat:fix/relaxng-compile-guards (ade2943) with main (2af1136)

Open in CodSpeed

Footnotes

  1. 32 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@gaborbernat
gaborbernat force-pushed the fix/relaxng-compile-guards branch 3 times, most recently from bb18ec2 to e513245 Compare October 1, 2026 16:43
Enforce the RELAX NG grammar restrictions at compile time so a malformed
or adversarial schema fails with a ValueError instead of crashing,
hanging, or exhausting memory when a document is validated.

- A <ref> with no name dereferenced a null attribute (SIGSEGV); reject
  it as the spec (4.10) requires.
- A reference cycle that never crosses an element hung or overflowed the
  stack; detect it with the 4.19 depth-stamp walk libxml2/jing/MSV use.
- An interleave whose branches share an element name or both match text
  violates 4.19/7.4 and drove the derivative into exponential memory;
  reject it at compile.
- A legal but ambiguous choice doubled the residual per child; hash-cons
  the derivative patterns and drop duplicate choice branches so it stays
  bounded.

The runtime ref-cycle guards are now unreachable and removed; the
compile check is the single source of cycle-freedom.
@gaborbernat
gaborbernat force-pushed the fix/relaxng-compile-guards branch from e513245 to ade2943 Compare October 1, 2026 16:51
@gaborbernat
gaborbernat merged commit ac1b184 into tox-dev:main Oct 1, 2026
48 of 52 checks passed
@gaborbernat
gaborbernat deleted the fix/relaxng-compile-guards branch October 1, 2026 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant