Repository navigation
Carry the Onigmo regexp compiler fixes into 1.9.3 through 2.7.8 - #11
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🔵 Needs a closer look
Security-sensitive C backports across five legacy runtimes warrant human validation of version compatibility and the reported build and regression results.
No open issues 🎉
What changed in this PR
Backports two regexp compiler fixes to five legacy Ruby build definitions, protecting against repeat-ID overflow and nested-repeat compilation hangs.
Changes:
- Adds self-contained patches applied before configuration, with version-specific context and error codes.
- Adds three installed-Ruby regression checks, including a deadline for nested-repeat compilation.
- Documents the fixes, affected versions, and why Ruby 1.8.7 needs no backport.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
test/build |
Adds regression checks for repeat IDs, null checks, and compilation hangs. |
README.md |
Documents regexp fixes and version coverage. |
2.7.8 |
Applies both compiler patches before configuration. |
2.5.9 |
Applies both compiler patches before configuration. |
2.3.8 |
Adds both patches with nonconflicting error codes. |
2.3.3 |
Adds both patches with nonconflicting error codes. |
1.9.3-p551 |
Adapts both patches to the older regexp engine. |
Review effort: Balanced
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c555be3e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two regexp compiler bugs are reachable from any pattern a user can get
into Regexp.new, and every Onigmo-based Ruby here has both:
- More than 32,767 range repeats or null checks wrap their signed 16-bit
ID negative, and a negative repeat ID indexes the match-time repeat
stack: Regexp.new("(?:foobar){0,100}" * 100000) =~ "x" segfaults
(ruby/ruby#16801, CVE-2026-110264).
- Nested bounded repeats overflow the int product that decides whether
to unroll a repeat, so Regexp.new("(?:.{90000,}){90000}") never
returns and ignores SIGTERM (ruby/ruby#19020).
Backport both as patch steps in each definition, applied to the
checksummed tarball with --fuzz=0 before configure. Inline heredocs, as
patch_lex_c99 does, so a definition still works fetched on its own. The
C is upstream's; only hunk context differs, plus the error codes on
2.3, whose Onigmo already spends -224 on ONIGERR_TOO_MANY_CAPTURE_GROUPS.
ruby/ruby#13332 and Bug #22384 don't apply: the double free they fix
came in with a later parse_subexp, and `|{2,1}` is already a plain
RegexpError here. 1.8.7 predates Onigmo; its engine caps quantifiers at
32,767 and a compiled pattern at 64 KB, and rejects all of these.
test/build now asserts each fix against the installed Ruby, so a patch
dropped from a step list, or one that applies and stops taking, fails
the build instead of shipping a Ruby that crashes on a hostile pattern.
ref: ruby/ruby#16801
ref: ruby/ruby#19020
ref: ruby/ruby#13332
ref: https://bugs.ruby-lang.org/issues/22384
The inline heredocs made each definition ~170 lines of C diff around ~40 lines of build recipe. Keep the patches as files in patches/<major.minor>/, which 2.3.3 and 2.3.8 share, and give each definition one apply_patches step that applies its series in name order, still with --fuzz=0 before configure. Each patch file opens with the explanation its heredoc had. A definition finds its series beside itself through DEFINITION_PATH, so it now needs the checkout. Fetched on its own, or piped in, it stops with an error before downloading anything rather than build a Ruby without the fixes. The README's single-file curl recipe now says it covers 1.8.7 only. The test images copy patches/ next to the definitions. The patch bodies are byte-identical to the heredocs they replace.
The one #19020 check compiled the nested repeat and counted any
RegexpError as a pass. That covered only the size-arithmetic half of
the patch: once the arithmetic is fixed, that pattern makes a small
program and never reaches the new compiled-program size cap, so a
backport without the cap still passed. And a broken backport that
rejected the pattern outright passed too.
Split it in two, each in a child with a 30 s deadline:
- The nested repeat is a valid pattern. On 1.9.3 and up it must compile
and not match "x"; only 1.8.7's engine may refuse it.
- 21475 repeats of .{100000,} must hit the size cap with "too big
compiled program" (1.8.7: "too big quantifier"). Unpatched, they
compile.
All four regexp checks pass on patched 1.8.7, 1.9.3, 2.3.8 and 2.7.8,
and all four fail on an unpatched 2.7.8.
jeremy
force-pushed
the
onigmo-regexp-overflow-fixes
branch
from
October 24, 2026 20:19
3c555be to
82318b9
Compare
Member
Author
|
Review pass on 82318b9:
|
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.
Backports two regexp compiler fixes into every Oniguruma or Onigmo Ruby here: 1.9.3-p551, 2.3.3, 2.3.8, 2.5.9 and 2.7.8. A pattern a user gets into
Regexp.newcan trigger either bug:Regexp.new("(?:foobar){0,100}" * 100000) =~ "x"segfaults.Regexp.new("(?:.{90000,}){90000}")never returns and ignores SIGTERM. Its second half caps the compiled program's size.The 3.3.13, 3.4.12 and 4.0.9 releases fix both upstream. None of the EOL Rubies will get a release.
How
patches/<major.minor>/(1.9,2.3,2.5,2.7; 2.3.3 and 2.3.8 share2.3), one file per fix, each opening with a header that says what it fixes and where it came from.apply_patchesbuild step. It applies its series in name order to the checksummed tarball with--fuzz=0, beforeconfigure.main, 206 inline, 59 now).test/buildgets four checks against the installed Ruby: both #16801 overflows, the nested repeat (it must compile promptly and not match), and the program-size cap. A patch that's dropped, or that applies but no longer takes, fails the build.Not backported:
parse_subexpintroduced. On these versions|{2,1}is already a plainRegexpError.RegexpError. The checks accept those errors on 1.8.7 only, so they run on every version.Verified
--fuzz=0under both BSD and GNUpatch. The patch files are byte-identical to the heredocs they replaced (10 of 10).too many range repeatandtoo many null check, the nested repeat compiles in about 0.1 s, and 21475 repeats of.{100000,}raisetoo big compiled program.make test-all TESTS=ruby/test_regexp.rbgives 0 failures and the same test counts as the unpatched builds.bin/cion 82318b9: lint clean,test/build all12 of 12 (Ubuntu noble arm64, Arch x86_64), signed off.test/build native allon 82318b9: 6 of 6 pass on macOS 27 arm64, the four regexp checks included.For review
ONIGERR_TOO_MANY_CAPTURE_GROUPS. Ruby shows only the message, so nothing visible changes.01-(#19020) has to come before02-(#16801).Not part of this change
-darwin27, with the CPU name missing. Worth a separate look.