Merge branch 'security-use-untrusted-regexp' into 'master'

Use UntrustedRegexp for CI refs matching

See merge request gitlab/gitlabhq!3005

6 files changed, 119 insertions, 46 deletions

diff --git a/spec/lib/gitlab/ci/build/policy/refs_spec.rb b/spec/lib/gitlab/ci/build/policy/refs_spec.rb
index b4ddbf89b70..ec0450643c3 100644
--- a/spec/lib/gitlab/ci/build/policy/refs_spec.rb
+++ b/spec/lib/gitlab/ci/build/policy/refs_spec.rb
@@ -92,10 +92,23 @@ describe Gitlab::Ci::Build::Policy::Refs do
           .to be_satisfied_by(pipeline)
       end
 
+      it 'is satisfied when case-insensitive regexp matches pipeline ref' do
+        expect(described_class.new(['/DOCS-.*/i']))
+          .to be_satisfied_by(pipeline)
+      end
+
       it 'is not satisfied when regexp does not match pipeline ref' do
         expect(described_class.new(['/fix-.*/']))
           .not_to be_satisfied_by(pipeline)
       end
     end
+
+    context 'malicious regexp' do
+      let(:pipeline) { build_stubbed(:ci_pipeline, ref: malicious_text) }
+
+      subject { described_class.new([malicious_regexp_ruby]) }
+
+      include_examples 'malicious regexp'
+    end
   end
 end
diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb
index 3ebc2e94727..cff7f57ceff 100644
--- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb
@@ -85,7 +85,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do
     end
 
     it 'raises error if evaluated regexp is not valid' do
-      allow(Gitlab::UntrustedRegexp).to receive(:valid?).and_return(true)
+      allow(Gitlab::UntrustedRegexp::RubySyntax).to receive(:valid?).and_return(true)
 
       regexp = described_class.new('/invalid ( .*/')
 
diff --git a/spec/lib/gitlab/ci/trace/stream_spec.rb b/spec/lib/gitlab/ci/trace/stream_spec.rb
index 38626f728d7..e45ea1c2528 100644
--- a/spec/lib/gitlab/ci/trace/stream_spec.rb
+++ b/spec/lib/gitlab/ci/trace/stream_spec.rb
@@ -414,7 +414,7 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do
 
       context 'malicious regexp' do
         let(:data) { malicious_text }
-        let(:regex) { malicious_regexp }
+        let(:regex) { malicious_regexp_re2 }
 
         include_examples 'malicious regexp'
       end
diff --git a/spec/lib/gitlab/route_map_spec.rb b/spec/lib/gitlab/route_map_spec.rb
index d672f7b5675..a39c774429e 100644
--- a/spec/lib/gitlab/route_map_spec.rb
+++ b/spec/lib/gitlab/route_map_spec.rb
@@ -60,7 +60,7 @@ describe Gitlab::RouteMap do
 
       subject do
         map = described_class.new(<<-"MAP".strip_heredoc)
-        - source: '#{malicious_regexp}'
+        - source: '#{malicious_regexp_re2}'
           public: '/'
         MAP
 
diff --git a/spec/lib/gitlab/untrusted_regexp/ruby_syntax_spec.rb b/spec/lib/gitlab/untrusted_regexp/ruby_syntax_spec.rb
new file mode 100644
index 00000000000..005d41580de
--- /dev/null
+++ b/spec/lib/gitlab/untrusted_regexp/ruby_syntax_spec.rb
@@ -0,0 +1,72 @@
+require 'fast_spec_helper'
+require 'support/shared_examples/malicious_regexp_shared_examples'
+
+describe Gitlab::UntrustedRegexp::RubySyntax do
+  describe '.matches_syntax?' do
+    it 'returns true if regexp is valid' do
+      expect(described_class.matches_syntax?('/some .* thing/'))
+        .to be true
+    end
+
+    it 'returns true if regexp is invalid, but resembles regexp' do
+      expect(described_class.matches_syntax?('/some ( thing/'))
+        .to be true
+    end
+  end
+
+  describe '.valid?' do
+    it 'returns true if regexp is valid' do
+      expect(described_class.valid?('/some .* thing/'))
+        .to be true
+    end
+
+    it 'returns false if regexp is invalid' do
+      expect(described_class.valid?('/some ( thing/'))
+        .to be false
+    end
+  end
+
+  describe '.fabricate' do
+    context 'when regexp is valid' do
+      it 'fabricates regexp without flags' do
+        expect(described_class.fabricate('/some .* thing/')).not_to be_nil
+      end
+    end
+
+    context 'when regexp is a raw pattern' do
+      it 'returns error' do
+        expect(described_class.fabricate('some .* thing')).to be_nil
+      end
+    end
+  end
+
+  describe '.fabricate!' do
+    context 'when regexp is using /regexp/ scheme with flags' do
+      it 'fabricates regexp with a single flag' do
+        regexp = described_class.fabricate!('/something/i')
+
+        expect(regexp).to eq Gitlab::UntrustedRegexp.new('(?i)something')
+        expect(regexp.scan('SOMETHING')).to be_one
+      end
+
+      it 'fabricates regexp with multiple flags' do
+        regexp = described_class.fabricate!('/something/im')
+
+        expect(regexp).to eq Gitlab::UntrustedRegexp.new('(?im)something')
+      end
+
+      it 'fabricates regexp without flags' do
+        regexp = described_class.fabricate!('/something/')
+
+        expect(regexp).to eq Gitlab::UntrustedRegexp.new('something')
+      end
+    end
+
+    context 'when regexp is a raw pattern' do
+      it 'raises an error' do
+        expect { described_class.fabricate!('some .* thing') }
+          .to raise_error(RegexpError)
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/untrusted_regexp_spec.rb b/spec/lib/gitlab/untrusted_regexp_spec.rb
index 0a6ac0aa294..9d483f13a5e 100644
--- a/spec/lib/gitlab/untrusted_regexp_spec.rb
+++ b/spec/lib/gitlab/untrusted_regexp_spec.rb
@@ -2,48 +2,6 @@ require 'fast_spec_helper'
 require 'support/shared_examples/malicious_regexp_shared_examples'
 
 describe Gitlab::UntrustedRegexp do
-  describe '.valid?' do
-    it 'returns true if regexp is valid' do
-      expect(described_class.valid?('/some ( thing/'))
-        .to be false
-    end
-
-    it 'returns true if regexp is invalid' do
-      expect(described_class.valid?('/some .* thing/'))
-        .to be true
-    end
-  end
-
-  describe '.fabricate' do
-    context 'when regexp is using /regexp/ scheme with flags' do
-      it 'fabricates regexp with a single flag' do
-        regexp = described_class.fabricate('/something/i')
-
-        expect(regexp).to eq described_class.new('(?i)something')
-        expect(regexp.scan('SOMETHING')).to be_one
-      end
-
-      it 'fabricates regexp with multiple flags' do
-        regexp = described_class.fabricate('/something/im')
-
-        expect(regexp).to eq described_class.new('(?im)something')
-      end
-
-      it 'fabricates regexp without flags' do
-        regexp = described_class.fabricate('/something/')
-
-        expect(regexp).to eq described_class.new('something')
-      end
-    end
-
-    context 'when regexp is a raw pattern' do
-      it 'raises an error' do
-        expect { described_class.fabricate('some .* thing') }
-          .to raise_error(RegexpError)
-      end
-    end
-  end
-
   describe '#initialize' do
     subject { described_class.new(pattern) }
 
@@ -92,11 +50,41 @@ describe Gitlab::UntrustedRegexp do
     end
   end
 
+  describe '#match?' do
+    subject { described_class.new(regexp).match?(text) }
+
+    context 'malicious regexp' do
+      let(:text) { malicious_text }
+      let(:regexp) { malicious_regexp_re2 }
+
+      include_examples 'malicious regexp'
+    end
+
+    context 'matching regexp' do
+      let(:regexp) { 'foo' }
+      let(:text) { 'foo' }
+
+      it 'returns an array of nil matches' do
+        is_expected.to eq(true)
+      end
+    end
+
+    context 'non-matching regexp' do
+      let(:regexp) { 'boo' }
+      let(:text) { 'foo' }
+
+      it 'returns an array of nil matches' do
+        is_expected.to eq(false)
+      end
+    end
+  end
+
   describe '#scan' do
     subject { described_class.new(regexp).scan(text) }
+
     context 'malicious regexp' do
       let(:text) { malicious_text }
-      let(:regexp) { malicious_regexp }
+      let(:regexp) { malicious_regexp_re2 }
 
       include_examples 'malicious regexp'
     end