Fix an infinite loop in Gitlab::UntrustedRegexp

RE2's scan implementation has a bug when passed an empty regexp, so
build our own using its `match` implementation instead.

2 files changed, 41 insertions, 8 deletions

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' }