Skip to content

Commit

Permalink
Remove ReDoS vulnerability
Browse files Browse the repository at this point in the history
Some regular expressions are vulnerable to Regular Expression Denial of
Service (ReDoS).

While the regexes are quite inefficient in the worst case, this does not
manifest as a DoS vulnerability in PHP because of the PHP regex
backtracking defaults
https://www.php.net/manual/en/pcre.configuration.php.
If a regular expression takes too long to process, PHP just bails and
says no match found even if the string would eventually match.

However, in ports such as the ruby version which relies on these regular
expressions, this manifests as a ReDoS vulnerability where a malicious
visitor can send a crafted long user agent header and trigger denial of
service.

Let's take:

Chrome/(\d+[\.\d]+).*MRCHROME

This regular expression is vulnerable to ReDoS due to the section:

(\d+[\.\d]+).*M

\d+ matches digits
[\.\d]+ matches digits (and '.')
.* matches digits (and everything else)

A malicious User-Agent string can be crafted containing
'Chrome/' followed by a long string of only digits:

Chrome/000000000000000000000000000000000000000000000000000
but with 3000+ zeroes

As this malicious string does NOT match the entire regular expression
(no MRCHROME), the regular expression matcher will backtrack and try
all N(N-1)/2 different ways of splitting the string of digits into the
three \d groups. Due to this backtracking, the runtime of the
malicious request will be approximately cubic with respect to N, so
doubling the length of the string of digits will make processing take
approximately 8 times longer.

There are some remaining with quadratic ReDoS but that can be fixed
another time if anybody cares. It requires much longer user-agent
strings to trigger an actual effect.

The fixed regexes are not 100% identical but I'm hoping that the test
coverage is sufficient to make sure I've not messed up here.
  • Loading branch information
bcaller committed Jan 3, 2023
1 parent 701351c commit 4566c49
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 15 deletions.
28 changes: 14 additions & 14 deletions regexes/client/browsers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@
default: 'Blink'

# Chromium GOST (https://github.com/deemru/chromium-gost)
- regex: 'Chrome/(\d+[\.\d]+).+\(Chromium GOST\)'
- regex: 'Chrome/(\d+\.[\.\d]+) .*\(Chromium GOST\)'
name: 'Chromium GOST'
version: '$1'
engine:
Expand Down Expand Up @@ -349,7 +349,7 @@
default: 'Gecko'

# CoolBrowser (https://play.google.com/store/apps/details?id=com.easybrowser.browser.coolbrowser)
- regex: 'Chrome/(\d+[\.\d]+).+AgentWeb.+UCBrowser'
- regex: 'Chrome/(\d+\.[\.\d]+) .*AgentWeb.+UCBrowser'
name: 'CoolBrowser'
version: '$1'
engine:
Expand Down Expand Up @@ -387,7 +387,7 @@
default: 'Blink'

# Qutebrowser (https://qutebrowser.org/)
- regex: 'qutebrowser/(\d+[\.\d]+).+Chrome'
- regex: 'qutebrowser/(\d+\.[\.\d]+) .*Chrome'
name: 'Qutebrowser'
version: '$1'
engine:
Expand Down Expand Up @@ -1232,7 +1232,7 @@
version: '$1'

#Amigo
- regex: 'Chrome/(\d+[\.\d]+).*MRCHROME'
- regex: 'Chrome/(\d+\.[\.\d]+) .*MRCHROME'
name: 'Amigo'
version: '$1'
engine:
Expand Down Expand Up @@ -1395,7 +1395,7 @@
default: 'Gecko'

#Swiftfox
- regex: 'Firefox/(\d+[\.\d]+).*\(Swiftfox\)'
- regex: 'Firefox/(\d+\.[\.\d]+) .*\(Swiftfox\)'
name: 'Swiftfox'
version: '$1'
engine:
Expand Down Expand Up @@ -1581,7 +1581,7 @@
version: '$1'
engine:
default: 'Blink'
- regex: 'Opera/(\d+[\.\d]+).+Opera Mobi'
- regex: 'Opera/(\d+\.[\.\d]+) .*Opera Mobi'
name: 'Opera Mobile'
version: '$1'
engine:
Expand Down Expand Up @@ -1616,7 +1616,7 @@
default: 'Presto'
versions:
15: 'Blink'
- regex: '(?:Opera|OPR)[/ ](?:9.80.*Version/)?(\d+[\.\d]+).+Edition Next'
- regex: '(?:Opera|OPR)[/ ](?:9.80.*Version/)?(\d+\.[\.\d]+) .*Edition Next'
name: 'Opera Next'
version: '$1'
engine:
Expand Down Expand Up @@ -1789,7 +1789,7 @@
default: 'Blink'

# Iron
- regex: 'Chrome(?:/(\d+[\.\d]+))?.*Iron'
- regex: 'Chrome(?:/(\d+\.[\.\d]+) )?.*Iron'
name: 'Iron'
version: '$1'
engine:
Expand Down Expand Up @@ -1957,7 +1957,7 @@
default: 'Blink'

# Web Explorer
- regex: 'Web Explorer/(\d+[\.\d]+).*Chrome'
- regex: 'Web Explorer/(\d+\.[\.\d]+) .*Chrome'
name: 'Web Explorer'
version: '$1'
engine:
Expand Down Expand Up @@ -2244,7 +2244,7 @@
- regex: '(?:Tizen|SLP) ?Browser(?:/(\d+[\.\d]+))?'
name: 'Tizen Browser'
version: '$1'
- regex: 'Tizen (?:\d+[\.\d]+).+ Version/(\d+[\.\d]+) (?:TV|Mobile|like)'
- regex: 'Tizen (?:\d+\.[\.\d]+)[^\.\d].* Version/(\d+[\.\d]+) (?:TV|Mobile|like)'
name: 'Tizen Browser'
version: '$1'
engine:
Expand Down Expand Up @@ -2413,7 +2413,7 @@
version: '$1'
engine:
default: 'WebKit'
- regex: 'GoogleEarth/(\d+[\.\d]+).+client:(?:Plus|Pro)'
- regex: 'GoogleEarth/(\d+\.[\.\d]+)[^\.\d].*client:(?:Plus|Pro)'
name: 'Google Earth Pro'
version: '$1'
engine:
Expand Down Expand Up @@ -2460,7 +2460,7 @@
version: '$1'
engine:
default: 'Trident'
- regex: 'MSIE (\d+[\.\d]+).*XBLWP7'
- regex: 'MSIE (\d+\.[\.\d]+)[^\.\d].*XBLWP7'
name: 'IE Mobile'
version: '$1'
engine:
Expand Down Expand Up @@ -2663,7 +2663,7 @@
version: '$1'
engine:
default: 'WebKit'
- regex: '(?:Version/(\d+[\.\d]+).*)?Mobile.*Safari/'
- regex: '(?:Version/(\d+\.[\.\d]+) .*)?Mobile.*Safari/'
name: 'Mobile Safari'
version: '$1'
engine:
Expand All @@ -2673,7 +2673,7 @@
version: ''
engine:
default: 'WebKit'
- regex: 'Version/(\d+[\.\d]+).*Safari/|(?:Safari|Safari(?:%20)?%E6%B5%8F%E8%A7%88%E5%99%A8)/?\d+'
- regex: 'Version/(\d+\.[\.\d]+) .*Safari/|(?:Safari|Safari(?:%20)?%E6%B5%8F%E8%A7%88%E5%99%A8)/?\d+'
name: 'Safari'
version: '$1'
engine:
Expand Down
2 changes: 1 addition & 1 deletion regexes/client/libraries.yml
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@
name: 'HTTPie'
version: '$1'

- regex: 'rest-client/(\d+[\.\d]+).*ruby'
- regex: 'rest-client/(\d+\.[\.\d]+) .*ruby'
name: 'REST Client for Ruby'
version: '$1'

Expand Down

0 comments on commit 4566c49

Please sign in to comment.