diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2019-02-25 15:41:52 +0100 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2019-03-15 14:38:28 +0100 |
commit | b22287f00fc10800486510c64139b4fefb38ac4c (patch) | |
tree | abcc545f4dafe74b9338a351dc3e095b1c82bef8 /spec | |
parent | 80fea82f3ab6afd486884020710eb01c06b048d9 (diff) | |
download | gitlab-ce-b22287f00fc10800486510c64139b4fefb38ac4c.tar.gz |
Make CI refs matching to to use UntrustedRegexp
This makes ref validation to use always `UntrustedRegexp`.
This also splits the existing RubySyntax into separate
class.
Diffstat (limited to 'spec')
-rw-r--r-- | spec/lib/gitlab/ci/build/policy/refs_spec.rb | 13 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/trace/stream_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/route_map_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/untrusted_regexp/ruby_syntax_spec.rb | 72 | ||||
-rw-r--r-- | spec/lib/gitlab/untrusted_regexp_spec.rb | 74 | ||||
-rw-r--r-- | spec/support/shared_examples/malicious_regexp_shared_examples.rb | 3 |
7 files changed, 121 insertions, 47 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 diff --git a/spec/support/shared_examples/malicious_regexp_shared_examples.rb b/spec/support/shared_examples/malicious_regexp_shared_examples.rb index db69b75c0c8..a86050e2cf2 100644 --- a/spec/support/shared_examples/malicious_regexp_shared_examples.rb +++ b/spec/support/shared_examples/malicious_regexp_shared_examples.rb @@ -2,7 +2,8 @@ require 'timeout' shared_examples 'malicious regexp' do let(:malicious_text) { 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!' } - let(:malicious_regexp) { '(?i)^(([a-z])+.)+[A-Z]([a-z])+$' } + let(:malicious_regexp_re2) { '(?i)^(([a-z])+.)+[A-Z]([a-z])+$' } + let(:malicious_regexp_ruby) { '/^(([a-z])+.)+[A-Z]([a-z])+$/i' } it 'takes under a second' do expect { Timeout.timeout(1) { subject } }.not_to raise_error |