[ruby-core:126860] [Ruby Bug#22384] Regexp compile error in the 2nd+ branch of a top-level alternation leaks the parse tree (regression from #13332, Ruby 4.0)
Issue #22384 has been reported by sethuarun (Sethupathi Arunachalam). ---------------------------------------- Bug #22384: Regexp compile error in the 2nd+ branch of a top-level alternation leaks the parse tree (regression from #13332, Ruby 4.0) https://bugs.ruby-lang.org/issues/22384 * Author: sethuarun (Sethupathi Arunachalam) * Status: Open * ruby -v: ruby 4.0.6 (2026-07-14 revision 03b6d3f889) +PRISM [x86_64-linux] * Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN ---------------------------------------- **Affects:** ruby 4.0.6 and 4.0.7 (official docker images and a jemalloc-linked build), master (commit still present). **Not affected:** 3.4.6. **Repro (plain Ruby, no extensions):** ```ruby def rss_kb = File.read('/proc/self/status')[/VmRSS:\s+(\d+)/, 1].to_i GC.start; b = rss_kb 500_000.times { Regexp.new("a|b(c") rescue nil } # RegexpError in the 2nd branch GC.start; puts "#{rss_kb - b} KB" # 4.0.6/4.0.7: ~65500 KB (134 B per compile); 3.4.6: 0 500_000.times { Regexp.new("b(c") rescue nil } # same error, no alternation: 0 KB on both ``` Also leaks: "a|b|c(d" (3rd branch), "a|b)c(d". Does not leak: nested "(a|b(c)". **Why it matters:** `Regexp#to_s` (rb_reg_str_with_term, re.c) re-runs onig_new on the inner pattern of any regexp whose source starts with "(?flags:" and ends with ")" to decide whether the prefix can be dropped. For every regexp shaped `/(?:a|b)c(d)/` that inner pattern is "a|b)c(d", which hits this leak. Rails calls Regexp#to_s on every Regexp in config.filter_parameters for every request (ActiveSupport::ParameterFilter compile_filters!, one ParameterFilter per request), and Rails precompiles filter_parameters into exactly that shape, so a Rails 7.1+/8.x app with a Regexp filter leaks ~128 B per request per process on Ruby 4.0. Observed on production: +3.4 MB/h per puma worker, 145 MB per worker after 40 h (jemalloc profile stacks: rb_reg_str_with_term → onig_new → onig_parse_make_tree → parse_subexp → parse_branch → parse_exp). **Cause:** commit 35000ac2ed "Prevent double free for too big repetition quantifiers (#13332)", regparse.c parse_subexp(). Before it, the alternation list was assigned to `*top` immediately and the loop's error paths called `onig_node_free(node)`. After it, the list lives in the local `topnode` and both error paths call `onig_node_free(topnode)` only. That is right for the `fetch_token` failure (`node` was already linked into topnode by the previous iteration), but wrong for the `parse_branch` failure: parse_branch returns the partially built branch in `*top` (see its `*top = node_new_list(node, NULL)` before the loop) and that node is not yet linked into topnode, so it is never freed. **Fix:** ```diff r = parse_branch(&node, tok, term, src, end, env); if (r < 0) { + onig_node_free(node); onig_node_free(topnode); return r; } ``` (parse_branch's own error path frees only the element that failed and leaves the list in *top, as it did before #13332; the top-level `if (r < 0) { onig_node_free(node); return r; }` a few lines above already does the same for the first branch.) -- https://bugs.ruby-lang.org/
Issue #22384 has been updated by sethuarun (Sethupathi Arunachalam). Fix and regression test: https://github.com/ruby/ruby/pull/19026. Verified on a build of v4.0.7 with the patch: Regexp.new("a|b(c") rescue nil goes from 134 B leaked per compile to 0, and 2M calls of /(?:a|b)c(d)/.to_s from +250 MB RSS to 0. Upstream Onigmo is not affected: the regression came from ruby/ruby commit 35000ac2ed (#13332), which was never applied there. ---------------------------------------- Bug #22384: Regexp compile error in the 2nd+ branch of a top-level alternation leaks the parse tree (regression from #13332, Ruby 4.0) https://bugs.ruby-lang.org/issues/22384#change-119219 * Author: sethuarun (Sethupathi Arunachalam) * Status: Open * ruby -v: ruby 4.0.6 (2026-07-14 revision 03b6d3f889) +PRISM [x86_64-linux] * Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN ---------------------------------------- **Affects:** ruby 4.0.6 and 4.0.7 (official docker images and a jemalloc-linked build), master (commit still present). **Not affected:** 3.4.6. **Repro (plain Ruby, no extensions):** ```ruby def rss_kb = File.read('/proc/self/status')[/VmRSS:\s+(\d+)/, 1].to_i GC.start; b = rss_kb 500_000.times { Regexp.new("a|b(c") rescue nil } # RegexpError in the 2nd branch GC.start; puts "#{rss_kb - b} KB" # 4.0.6/4.0.7: ~65500 KB (134 B per compile); 3.4.6: 0 500_000.times { Regexp.new("b(c") rescue nil } # same error, no alternation: 0 KB on both ``` Also leaks: "a|b|c(d" (3rd branch), "a|b)c(d". Does not leak: nested "(a|b(c)". **Why it matters:** `Regexp#to_s` (rb_reg_str_with_term, re.c) re-runs onig_new on the inner pattern of any regexp whose source starts with "(?flags:" and ends with ")" to decide whether the prefix can be dropped. For every regexp shaped `/(?:a|b)c(d)/` that inner pattern is "a|b)c(d", which hits this leak. Rails calls Regexp#to_s on every Regexp in config.filter_parameters for every request (ActiveSupport::ParameterFilter compile_filters!, one ParameterFilter per request), and Rails precompiles filter_parameters into exactly that shape, so a Rails 7.1+/8.x app with a Regexp filter leaks ~128 B per request per process on Ruby 4.0. Observed on production: +3.4 MB/h per puma worker, 145 MB per worker after 40 h (jemalloc profile stacks: rb_reg_str_with_term → onig_new → onig_parse_make_tree → parse_subexp → parse_branch → parse_exp). **Cause:** commit 35000ac2ed "Prevent double free for too big repetition quantifiers (#13332)", regparse.c parse_subexp(). Before it, the alternation list was assigned to `*top` immediately and the loop's error paths called `onig_node_free(node)`. After it, the list lives in the local `topnode` and both error paths call `onig_node_free(topnode)` only. That is right for the `fetch_token` failure (`node` was already linked into topnode by the previous iteration), but wrong for the `parse_branch` failure: parse_branch returns the partially built branch in `*top` (see its `*top = node_new_list(node, NULL)` before the loop) and that node is not yet linked into topnode, so it is never freed. **Fix:** ```diff r = parse_branch(&node, tok, term, src, end, env); if (r < 0) { + onig_node_free(node); onig_node_free(topnode); return r; } ``` (parse_branch's own error path frees only the element that failed and leaves the list in *top, as it did before #13332; the top-level `if (r < 0) { onig_node_free(node); return r; }` a few lines above already does the same for the first branch.) -- https://bugs.ruby-lang.org/
Issue #22384 has been updated by k0kubun (Takashi Kokubun). Backport changed from 3.3: DONTNEED, 3.4: DONTNEED, 4.0: REQUIRED to 3.3: DONTNEED, 3.4: DONTNEED, 4.0: DONE ruby_4_0 commit:f42603547885d3455d09f1bc7fda1d552ebd4fd9. ---------------------------------------- Bug #22384: Regexp compile error in the 2nd+ branch of a top-level alternation leaks the parse tree (regression from #13332, Ruby 4.0) https://bugs.ruby-lang.org/issues/22384#change-119449 * Author: sethuarun (Sethupathi Arunachalam) * Status: Closed * ruby -v: ruby 4.0.6 (2026-07-14 revision 03b6d3f889) +PRISM [x86_64-linux] * Backport: 3.3: DONTNEED, 3.4: DONTNEED, 4.0: DONE ---------------------------------------- **Affects:** ruby 4.0.6 and 4.0.7 (official docker images and a jemalloc-linked build), master (commit still present). **Not affected:** 3.4.6. **Repro (plain Ruby, no extensions):** ```ruby def rss_kb = File.read('/proc/self/status')[/VmRSS:\s+(\d+)/, 1].to_i GC.start; b = rss_kb 500_000.times { Regexp.new("a|b(c") rescue nil } # RegexpError in the 2nd branch GC.start; puts "#{rss_kb - b} KB" # 4.0.6/4.0.7: ~65500 KB (134 B per compile); 3.4.6: 0 500_000.times { Regexp.new("b(c") rescue nil } # same error, no alternation: 0 KB on both ``` Also leaks: "a|b|c(d" (3rd branch), "a|b)c(d". Does not leak: nested "(a|b(c)". **Why it matters:** `Regexp#to_s` (rb_reg_str_with_term, re.c) re-runs onig_new on the inner pattern of any regexp whose source starts with "(?flags:" and ends with ")" to decide whether the prefix can be dropped. For every regexp shaped `/(?:a|b)c(d)/` that inner pattern is "a|b)c(d", which hits this leak. Rails calls Regexp#to_s on every Regexp in config.filter_parameters for every request (ActiveSupport::ParameterFilter compile_filters!, one ParameterFilter per request), and Rails precompiles filter_parameters into exactly that shape, so a Rails 7.1+/8.x app with a Regexp filter leaks ~128 B per request per process on Ruby 4.0. Observed on production: +3.4 MB/h per puma worker, 145 MB per worker after 40 h (jemalloc profile stacks: rb_reg_str_with_term → onig_new → onig_parse_make_tree → parse_subexp → parse_branch → parse_exp). **Cause:** commit 35000ac2ed "Prevent double free for too big repetition quantifiers (#13332)", regparse.c parse_subexp(). Before it, the alternation list was assigned to `*top` immediately and the loop's error paths called `onig_node_free(node)`. After it, the list lives in the local `topnode` and both error paths call `onig_node_free(topnode)` only. That is right for the `fetch_token` failure (`node` was already linked into topnode by the previous iteration), but wrong for the `parse_branch` failure: parse_branch returns the partially built branch in `*top` (see its `*top = node_new_list(node, NULL)` before the loop) and that node is not yet linked into topnode, so it is never freed. **Fix:** ```diff r = parse_branch(&node, tok, term, src, end, env); if (r < 0) { + onig_node_free(node); onig_node_free(topnode); return r; } ``` (parse_branch's own error path frees only the element that failed and leaves the list in *top, as it did before #13332; the top-level `if (r < 0) { onig_node_free(node); return r; }` a few lines above already does the same for the first branch.) -- https://bugs.ruby-lang.org/
participants (2)
-
k0kubun (Takashi Kokubun) -
sethuarun (Sethupathi Arunachalam)