summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2019-01-24 12:50:31 +0000
committerYorick Peterse <yorickpeterse@gmail.com>2019-01-24 12:50:34 +0000
commit0b7db931af324b7bf1fe4faab821268c29bb7118 (patch)
tree3c66eb9945664286969029d2c2668d884d8bddce
parent7f0ce1ea578469615885d4a4707bd204d4c4493a (diff)
downloadgitlab-ce-0b7db931af324b7bf1fe4faab821268c29bb7118.tar.gz
Merge branch 'security-fix-regex-dos-11-5' into 'security-11-5'
[11.5] Fix DoS in reference extraction regexes See merge request gitlab/gitlabhq!2779 (cherry picked from commit 9f3dc81480d4b72a201e3517335c4f18235a1f7d) 0a37ec23 Fix slow project reference pattern regex
-rw-r--r--app/models/project.rb1
-rw-r--r--changelogs/unreleased/security-fix-regex-dos.yml5
-rw-r--r--lib/gitlab/path_regex.rb3
-rw-r--r--spec/lib/banzai/filter/project_reference_filter_spec.rb6
4 files changed, 14 insertions, 1 deletions
diff --git a/app/models/project.rb b/app/models/project.rb
index 68a441f72fe..7dbba9c0f00 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -492,6 +492,7 @@ class Project < ActiveRecord::Base
def reference_pattern
%r{
+ (?<!#{Gitlab::PathRegex::PATH_START_CHAR})
((?<namespace>#{Gitlab::PathRegex::FULL_NAMESPACE_FORMAT_REGEX})\/)?
(?<project>#{Gitlab::PathRegex::PROJECT_PATH_FORMAT_REGEX})
}x
diff --git a/changelogs/unreleased/security-fix-regex-dos.yml b/changelogs/unreleased/security-fix-regex-dos.yml
new file mode 100644
index 00000000000..b08566d2f15
--- /dev/null
+++ b/changelogs/unreleased/security-fix-regex-dos.yml
@@ -0,0 +1,5 @@
+---
+title: Fix slow regex in project reference pattern
+merge_request:
+author:
+type: security
diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb
index fa68dead80b..3c888be0710 100644
--- a/lib/gitlab/path_regex.rb
+++ b/lib/gitlab/path_regex.rb
@@ -125,7 +125,8 @@ module Gitlab
# allow non-regex validations, etc), `NAMESPACE_FORMAT_REGEX_JS` serves as a Javascript-compatible version of
# `NAMESPACE_FORMAT_REGEX`, with the negative lookbehind assertion removed. This means that the client-side validation
# will pass for usernames ending in `.atom` and `.git`, but will be caught by the server-side validation.
- PATH_REGEX_STR = '[a-zA-Z0-9_\.][a-zA-Z0-9_\-\.]*'.freeze
+ PATH_START_CHAR = '[a-zA-Z0-9_\.]'.freeze
+ PATH_REGEX_STR = PATH_START_CHAR + '[a-zA-Z0-9_\-\.]*'.freeze
NAMESPACE_FORMAT_REGEX_JS = PATH_REGEX_STR + '[a-zA-Z0-9_\-]|[a-zA-Z0-9_]'.freeze
NO_SUFFIX_REGEX = /(?<!\.git|\.atom)/.freeze
diff --git a/spec/lib/banzai/filter/project_reference_filter_spec.rb b/spec/lib/banzai/filter/project_reference_filter_spec.rb
index 48140305e26..060a680a996 100644
--- a/spec/lib/banzai/filter/project_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/project_reference_filter_spec.rb
@@ -26,6 +26,12 @@ describe Banzai::Filter::ProjectReferenceFilter do
expect(reference_filter(act).to_html).to eq(CGI.escapeHTML(exp))
end
+ it 'fails fast for long invalid string' do
+ expect do
+ Timeout.timeout(5.seconds) { reference_filter("A" * 50000).to_html }
+ end.not_to raise_error
+ end
+
it 'allows references with text after the > character' do
doc = reference_filter("Hey #{reference}foo")
expect(doc.css('a').first.attr('href')).to eq urls.project_url(subject)