Skip to content

Regexp.timeout= is stored but never applied, and a fired limit returns nil instead of raising Regexp::TimeoutError (fail-open for validators) #776

Closed

Description

@tannevaled

Summary

Two separate defects in the same defence, both of which make a regexp used as a validator fail open:

  1. Regexp.timeout= is stored, echoed back, and never applied. The class-level setter is Ruby 3.2's documented ReDoS defence. Setting it changes nothing, and Regexp.timeout faithfully reports the value you set — so a host that configured it has no way to notice it is inert.
  2. When a limit does fire, the match returns nil instead of raising. Regexp::TimeoutError can never be rescued. For a regexp used to validate or to deny, "timed out" is therefore indistinguishable from "does not match", so a crafted subject passes the check.

Found during the security audit tracked in #768.

Measurement

p1 = Regexp.new("(a+)+\\1b"); s = "a"*26
puts "Regexp.timeout initially: #{Regexp.timeout.inspect}"
# A: no timeout at all
Regexp.timeout = 0.05
puts "reads back: #{Regexp.timeout.inspect}"
# B: class-level
p2 = Regexp.new("(a+)+\\1b", timeout: 0.05)
# C: per-Regexp
rbgo MRI 4.0.5
Regexp.timeout initially nil nil
A no timeout nil after 0.994 s, no exception nil after 1.775 s, no exception
reads back after = 0.05 0.05 0.05
B class-level Regexp.timeout = 0.05 nil after 0.843 s, no exception Regexp::TimeoutError after 0.050 s
C per-Regexp timeout: 0.05 nil after 0.050 s, no exception Regexp::TimeoutError after 0.050 s

Row C is what makes this readable rather than a guess about an absence: the per-regexp option is demonstrably enforced — 0.050 s, to the millisecond, against 0.843 s unlimited — which proves the probe shape works and the plumbing exists. So row B's 0.843 s is a measurement of the class-level setter being ignored, 17× over its own limit, and not a probe that failed to arrive.

Row C also shows the second defect on its own: the limit fired, and the answer was nil.

Where, as far as I have read

  • Only the per-Regexp timeout: option is threaded through to the engine's WithTimeout in internal/vm/regexp.go. The class-level setter has a reader and a writer and no consumer.
  • go-regexp/engine/regexp.go collapses the two outcomes: the match path is if err != nil || !matched { — so ErrTimeout and ErrBudget both become a clean non-match rather than an error the VM could turn into Regexp::TimeoutError.

Worth saying, because it changes what needs fixing: the engine is not defenceless. It is a hand-written backtracking VM (go-regexp/engine, not RE2), but with a step budget, a wall-clock deadline, a \g<> call-depth cap and (pc,sp) memoisation that makes the RE2-compatible subset linear. The classic one-line ReDoS does not reproduce: /(a+)+b/ =~ "a"*40 is 0.001 s. Measured against MRI on the backreference form, which memoisation correctly cannot help, rbgo is better: at "a"*26 rbgo takes 0.860 s where MRI takes 5.032 s, and at "a"*30 rbgo finishes in 2.257 s where MRI is OOM-killed at 2 GB.

So this is not "rbgo is open to ReDoS". It is that the limits which exist are not reportable, and the one knob a host is told to use is not wired up.

Suggested shape

  1. Thread the class-level Regexp.timeout to the engine, or make the setter raise NotImplementedError so a host finds out at configuration time instead of never. Silently accepting a security setting is the worse of the two.
  2. Separate ErrTimeout/ErrBudget from "no match" at the if err != nil || !matched site and raise Regexp::TimeoutError. Even if the class-level setter stays unimplemented, this alone turns a fail-open into a fail-closed.
  3. A test asserting that a timed-out match raises rather than returning nil — the current behaviour is exactly the one a validator cannot distinguish from success.

🤖 Generated with Claude Code

Activity

  1. tannevaled commented on Oct 5, 2026

    @tannevaled
    ContributorAuthor

    Correcting two things in the issue body — both mine, and the second changes the severity

    1. "rbgo beats MRI" was not a speed result

    The body says, as a mitigating note:

    Measured against MRI on the backreference form, which memoisation correctly cannot help, rbgo is better: at "a"*26 rbgo takes 0.860 s where MRI takes 5.032 s, and at "a"*30 rbgo finishes in 2.257 s where MRI is OOM-killed at 2 GB.

    rbgo did not finish. It reached the step budget and reported nil. I measured wall-clock time on a call whose failure was being swallowed, and read the silence as a win — the engine's hardening is real, but on that input it bought a silent refusal, not a faster answer. MRI's 1.7–5 s on the same subject is an exhaustive search; MRI has no step budget.

    This is the proxy-measure trap in its plainest form: the quantity I timed was not the quantity I was claiming.

    2. The fail-open did not need Regexp.timeout to be configured — it was the default

    Row A of the table says "no timeout: nil after 0.994 s", which I presented as the unlimited baseline against which rows B and C were judged. Measured at the engine boundary, that row was already the budget firing:

    subject, (a+)+\1b result
    "a"*20 0.166 s, search completed, genuinely no match
    "a"*22 0.688 s, search completed, genuinely no match
    "a"*24 0.873 s, budget exhausted
    "a"*26 0.829 s, budget exhausted

    So the issue understated itself. I framed this as "the limits that exist are not reportable", with the implication that a host had to configure something before the fail-open was reachable. It was reachable at default settings, with no timeout set at all — which makes it a plain fail-open in the default configuration rather than a misconfiguration hazard.

    Both corrections come from #790's own measurements, which I verified before accepting: on main the no-timeout case answers nil after 0.834 s, and a matching subject still matches in 0.000 s on both binaries, so the change only affects subjects that exhaust the budget.

    On the class, since I nearly objected to it

    I was going to argue that budget exhaustion should raise Regexp::TimeoutError, because that is the class a defensive program rescues. Reading MRI first settled it the other way: Regexp::TimeoutError is defined under RegexpError (re.c:4862), and an onig resource error is a plain RegexpError (re.c:1724-1728). So MRI's own taxonomy separates "ran out of time" from "ran out of resource", and RegexpError for the budget is the conformant choice. Noted here because the reasoning is worth more than the conclusion.

    🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions