From: "Dan0042 (Daniel DeLorme) via ruby-core" <ruby-core@...>
Date: 2023-09-14T16:23:39+00:00
Subject: [ruby-core:114758] [Ruby master Feature#19720] Warning for non-linear Regexps

Issue #19720 has been updated by Dan0042 (Daniel DeLorme).


I agree with Matz, false positives are worse than no warnings at all. I use atomic groups and positive/negative lookahead/lookbehind all the time. In particular, atomic groups are a great tool to *prevent* ReDoS-susceptible regexps, so to warn against them would be really counterproductive.

But I think this "non-linear" flag could serve as a good first line of defense, by enabling a timeout by default. And "linear" regexps don't need the overhead of a timeout.

----------------------------------------
Feature #19720: Warning for non-linear Regexps
https://bugs.ruby-lang.org/issues/19720#change-104595

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
----------------------------------------
I believe the best way to solve ReDoS is to ensure all Regexps used in the process are linear.
Using `Regexp.timeout = 5.0` or so does not really prevent ReDoS, given enough requests causing that timeout the servers will still be very unresponsive.

To this purpose, we should make it easy to identify non-linear Regexps and fix them.

I suggest we either use
1. a performance warning (enabled with `Warning[:performance] = true`, #19538) or
2. a new regexp warning category (enabled with `Warning[:regexp] = true`).

I think we should warn only once per non-linear Regexp, to avoid too many such warnings.
We could warn as soon as the Regexp is created, or on first match.
On first match might makes more sense for Ruby implementations which compile the Regexp lazily (since that is costly during startup), and also avoids warning for Regexps which are never used (which can be good or bad).
OTOH, if the warning is enabled, we could always compile the Regexp eagerly (or at least checks whether it's linear), and that would then provide a better way to guarantee that all Regexps created so far are linear.

Because warnings are easily customizable, it is also possible to e.g. `raise/abort` on such a warning, if one wants to ensure their application does not use a non-linear Regexp and so cannot be vulnerable to ReDoS:
```ruby
Warning.extend Module.new {
  def warn(message, category: nil, **)
    raise message if category == :regexp
    super
  end
}
```
A regexp warning category seems better for that as it makes it easy to filter by category, if a performance warning one would need to match the message which is less clean.

As a note, TruffleRuby already has a similar warning, as a command-line option:
```
$ truffleruby --experimental-options --warn-truffle-regex-compile-fallback -e 'Gem'
truffleruby-dev/lib/mri/rubygems/version.rb:176: warning: Regexp /\A\s*([0-9]+(?>\.[0-9a-zA-Z]+)*(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?)?\s*\z/ at_start=false encoding=US-ASCII requires backtracking and will not match in linear time
truffleruby-dev/lib/mri/rubygems/requirement.rb:105: warning: Regexp /\A\s*(=|!=|>|<|>=|<=|~>)?\s*([0-9]+(?>\.[0-9a-zA-Z]+)*(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?)\s*\z/ at_start=false encoding=US-ASCII requires backtracking and will not match in linear time
```

So the warning message could be like
`FILE:LINE: warning: Regexp /REGEXP/ requires backtracking and might not match in linear time and might cause ReDoS`
or more concise:
`FILE:LINE: warning: Regexp /REGEXP/ requires backtracking and might cause ReDoS`



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/