Merge branch 'fix-re2-infinite-loop-nick' into 'security-9-3'

Fix an infinite loop in Gitlab:UntrustedRegexp

See merge request !2146

3 files changed, 45 insertions, 8 deletions

diff --git a/changelogs/unreleased/fix-re2-infinite-loop-nick.yml b/changelogs/unreleased/fix-re2-infinite-loop-nick.yml
new file mode 100644
index 00000000000..3c314ab2718
--- /dev/null
+++ b/changelogs/unreleased/fix-re2-infinite-loop-nick.yml
@@ -0,0 +1,4 @@
+---
+title: Fix an infinite loop when handling user-supplied regular expressions
+merge_request:
+author:
diff --git a/lib/gitlab/untrusted_regexp.rb b/lib/gitlab/untrusted_regexp.rb
index 8b43f0053d6..925b2158a22 100644
--- a/lib/gitlab/untrusted_regexp.rb
+++ b/lib/gitlab/untrusted_regexp.rb
@@ -22,13 +22,28 @@ module Gitlab
     end
 
     def scan(text)
-      scan_regexp.scan(text).map do |match|
-        if regexp.number_of_capturing_groups == 0
-          match.first
-        else
-          match
-        end
+      text = text.dup # modified in-place
+      results = []
+
+      loop do
+        match = scan_regexp.match(text)
+        break unless match
+
+        # Ruby scan returns empty strings, not nil
+        groups = match.to_a.map(&:to_s)
+
+        results << 
+          if regexp.number_of_capturing_groups.zero?
+            groups[0]
+          else
+            groups[1..-1]
+          end
+
+        text.slice!(0, match.end(0) || 1)
+        break unless text.present?
       end
+
+      results
     end
 
     def replace(text, rewrite)
@@ -43,7 +58,7 @@ module Gitlab
     # groups, so work around it
     def scan_regexp
       @scan_regexp ||=
-        if regexp.number_of_capturing_groups == 0
+        if regexp.number_of_capturing_groups.zero?
           RE2::Regexp.new('(' + regexp.source + ')')
         else
           regexp
diff --git a/spec/lib/gitlab/untrusted_regexp_spec.rb b/spec/lib/gitlab/untrusted_regexp_spec.rb
index 66045917cb3..a2ef2a27e4c 100644
--- a/spec/lib/gitlab/untrusted_regexp_spec.rb
+++ b/spec/lib/gitlab/untrusted_regexp_spec.rb
@@ -46,10 +46,28 @@ describe Gitlab::UntrustedRegexp do
     context 'malicious regexp' do
       let(:text) { malicious_text }
       let(:regexp) { malicious_regexp }
- 
+
       include_examples 'malicious regexp'
     end
 
+    context 'empty regexp' do
+      let(:regexp) { '' }
+      let(:text) { 'foo' }
+
+      it 'returns an array of empty matches' do
+        is_expected.to eq(['', '', ''])
+      end
+    end
+
+    context 'empty capture group regexp' do
+      let(:regexp) { '()' }
+      let(:text) { 'foo' }
+
+      it 'returns arrays of empty matches in an array' do
+        is_expected.to eq([[''], [''], ['']])
+      end
+    end
+
     context 'no capture group' do
       let(:regexp) { '.+' }
       let(:text) { 'foo' }