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

Fix an infinite loop in Gitlab:UntrustedRegexp

See merge request !2146

changelogs/unreleased/fix-re2-infinite-loop-nick.yml

+---
+title: Fix an infinite loop when handling user-supplied regular expressions
+merge_request:
+author:

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

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