Commit c07cf3b6 authored by Kamil Trzciński's avatar Kamil Trzciński

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.
parent 6a7facfd
---
title: Use UntrustedRegexp for matching refs policy
merge_request:
author:
type: security
...@@ -340,6 +340,19 @@ job: ...@@ -340,6 +340,19 @@ job:
- branches - branches
``` ```
Pattern matching is case-sensitive by default. Use `i` flag modifier, like
`/pattern/i` to make a pattern case-insensitive:
```yaml
job:
# use regexp
only:
- /^issue-.*$/i
# use special keyword
except:
- branches
```
In this example, `job` will run only for refs that are tagged, or if a build is In this example, `job` will run only for refs that are tagged, or if a build is
explicitly requested via an API trigger or a [Pipeline Schedule][schedules]: explicitly requested via an API trigger or a [Pipeline Schedule][schedules]:
......
...@@ -35,8 +35,8 @@ module Gitlab ...@@ -35,8 +35,8 @@ module Gitlab
# patterns can be matched only when branch or tag is used # patterns can be matched only when branch or tag is used
# the pattern matching does not work for merge requests pipelines # the pattern matching does not work for merge requests pipelines
if pipeline.branch? || pipeline.tag? if pipeline.branch? || pipeline.tag?
if pattern.first == "/" && pattern.last == "/" if regexp = Gitlab::UntrustedRegexp::RubySyntax.fabricate(pattern)
Regexp.new(pattern[1...-1]) =~ pipeline.ref regexp.match?(pipeline.ref)
else else
pattern == pipeline.ref pattern == pipeline.ref
end end
......
...@@ -13,13 +13,13 @@ module Gitlab ...@@ -13,13 +13,13 @@ module Gitlab
def initialize(regexp) def initialize(regexp)
@value = regexp @value = regexp
unless Gitlab::UntrustedRegexp.valid?(@value) unless Gitlab::UntrustedRegexp::RubySyntax.valid?(@value)
raise Lexer::SyntaxError, 'Invalid regular expression!' raise Lexer::SyntaxError, 'Invalid regular expression!'
end end
end end
def evaluate(variables = {}) def evaluate(variables = {})
Gitlab::UntrustedRegexp.fabricate(@value) Gitlab::UntrustedRegexp::RubySyntax.fabricate!(@value)
rescue RegexpError rescue RegexpError
raise Expression::RuntimeError, 'Invalid regular expression!' raise Expression::RuntimeError, 'Invalid regular expression!'
end end
......
...@@ -45,17 +45,15 @@ module Gitlab ...@@ -45,17 +45,15 @@ module Gitlab
end end
def validate_regexp(value) def validate_regexp(value)
!value.nil? && Regexp.new(value.to_s) && true Gitlab::UntrustedRegexp::RubySyntax.valid?(value)
rescue RegexpError, TypeError
false
end end
def validate_string_or_regexp(value) def validate_string_or_regexp(value)
return true if value.is_a?(Symbol) return true if value.is_a?(Symbol)
return false unless value.is_a?(String) return false unless value.is_a?(String)
if value.first == '/' && value.last == '/' if Gitlab::UntrustedRegexp::RubySyntax.matches_syntax?(value)
validate_regexp(value[1...-1]) validate_regexp(value)
else else
true true
end end
......
...@@ -120,17 +120,13 @@ module Gitlab ...@@ -120,17 +120,13 @@ module Gitlab
private private
def look_like_regexp?(value) def matches_syntax?(value)
value.is_a?(String) && value.start_with?('/') && Gitlab::UntrustedRegexp::RubySyntax.matches_syntax?(value)
value.end_with?('/')
end end
def validate_regexp(value) def validate_regexp(value)
look_like_regexp?(value) && matches_syntax?(value) &&
Regexp.new(value.to_s[1...-1]) && Gitlab::UntrustedRegexp::RubySyntax.valid?(value)
true
rescue RegexpError
false
end end
end end
...@@ -149,7 +145,7 @@ module Gitlab ...@@ -149,7 +145,7 @@ module Gitlab
def validate_string_or_regexp(value) def validate_string_or_regexp(value)
return false unless value.is_a?(String) return false unless value.is_a?(String)
return validate_regexp(value) if look_like_regexp?(value) return validate_regexp(value) if matches_syntax?(value)
true true
end end
......
...@@ -35,6 +35,10 @@ module Gitlab ...@@ -35,6 +35,10 @@ module Gitlab
matches matches
end end
def match?(text)
text.present? && scan(text).present?
end
def replace(text, rewrite) def replace(text, rewrite)
RE2.Replace(text, regexp, rewrite) RE2.Replace(text, regexp, rewrite)
end end
...@@ -43,37 +47,6 @@ module Gitlab ...@@ -43,37 +47,6 @@ module Gitlab
self.source == other.source self.source == other.source
end end
# Handles regular expressions with the preferred RE2 library where possible
# via UntustedRegex. Falls back to Ruby's built-in regular expression library
# when the syntax would be invalid in RE2.
#
# One difference between these is `(?m)` multi-line mode. Ruby regex enables
# this by default, but also handles `^` and `$` differently.
# See: https://www.regular-expressions.info/modifiers.html
def self.with_fallback(pattern, multiline: false)
UntrustedRegexp.new(pattern, multiline: multiline)
rescue RegexpError
Regexp.new(pattern)
end
def self.valid?(pattern)
!!self.fabricate(pattern)
rescue RegexpError
false
end
def self.fabricate(pattern)
matches = pattern.match(%r{^/(?<regexp>.+)/(?<flags>[ismU]*)$})
raise RegexpError, 'Invalid regular expression!' if matches.nil?
expression = matches[:regexp]
flags = matches[:flags]
expression.prepend("(?#{flags})") if flags.present?
self.new(expression, multiline: false)
end
private private
attr_reader :regexp attr_reader :regexp
......
# frozen_string_literal: true
module Gitlab
class UntrustedRegexp
# This class implements support for Ruby syntax of regexps
# and converts that to RE2 representation:
# /<regexp>/<flags>
class RubySyntax
PATTERN = %r{^/(?<regexp>.+)/(?<flags>[ismU]*)$}.freeze
# Checks if pattern matches a regexp pattern
# but does not enforce it's validity
def self.matches_syntax?(pattern)
pattern.is_a?(String) && pattern.match(PATTERN).present?
end
# The regexp can match the pattern `/.../`, but may not be fabricatable:
# it can be invalid or incomplete: `/match ( string/`
def self.valid?(pattern)
!!self.fabricate(pattern)
end
def self.fabricate(pattern)
self.fabricate!(pattern)
rescue RegexpError
nil
end
def self.fabricate!(pattern)
raise RegexpError, 'Pattern is not string!' unless pattern.is_a?(String)
matches = pattern.match(PATTERN)
raise RegexpError, 'Invalid regular expression!' if matches.nil?
expression = matches[:regexp]
flags = matches[:flags]
expression.prepend("(?#{flags})") if flags.present?
UntrustedRegexp.new(expression, multiline: false)
end
end
end
end
...@@ -92,10 +92,23 @@ describe Gitlab::Ci::Build::Policy::Refs do ...@@ -92,10 +92,23 @@ describe Gitlab::Ci::Build::Policy::Refs do
.to be_satisfied_by(pipeline) .to be_satisfied_by(pipeline)
end 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 it 'is not satisfied when regexp does not match pipeline ref' do
expect(described_class.new(['/fix-.*/'])) expect(described_class.new(['/fix-.*/']))
.not_to be_satisfied_by(pipeline) .not_to be_satisfied_by(pipeline)
end end
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
end end
...@@ -85,7 +85,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do ...@@ -85,7 +85,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do
end end
it 'raises error if evaluated regexp is not valid' do 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 ( .*/') regexp = described_class.new('/invalid ( .*/')
......
...@@ -414,7 +414,7 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do ...@@ -414,7 +414,7 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do
context 'malicious regexp' do context 'malicious regexp' do
let(:data) { malicious_text } let(:data) { malicious_text }
let(:regex) { malicious_regexp } let(:regex) { malicious_regexp_re2 }
include_examples 'malicious regexp' include_examples 'malicious regexp'
end end
......
...@@ -60,7 +60,7 @@ describe Gitlab::RouteMap do ...@@ -60,7 +60,7 @@ describe Gitlab::RouteMap do
subject do subject do
map = described_class.new(<<-"MAP".strip_heredoc) map = described_class.new(<<-"MAP".strip_heredoc)
- source: '#{malicious_regexp}' - source: '#{malicious_regexp_re2}'
public: '/' public: '/'
MAP MAP
......
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
...@@ -2,48 +2,6 @@ require 'fast_spec_helper' ...@@ -2,48 +2,6 @@ require 'fast_spec_helper'
require 'support/shared_examples/malicious_regexp_shared_examples' require 'support/shared_examples/malicious_regexp_shared_examples'
describe Gitlab::UntrustedRegexp do 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 describe '#initialize' do
subject { described_class.new(pattern) } subject { described_class.new(pattern) }
...@@ -92,11 +50,41 @@ describe Gitlab::UntrustedRegexp do ...@@ -92,11 +50,41 @@ describe Gitlab::UntrustedRegexp do
end end
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 describe '#scan' do
subject { described_class.new(regexp).scan(text) } subject { described_class.new(regexp).scan(text) }
context 'malicious regexp' do context 'malicious regexp' do
let(:text) { malicious_text } let(:text) { malicious_text }
let(:regexp) { malicious_regexp } let(:regexp) { malicious_regexp_re2 }
include_examples 'malicious regexp' include_examples 'malicious regexp'
end end
......
...@@ -2,7 +2,8 @@ require 'timeout' ...@@ -2,7 +2,8 @@ require 'timeout'
shared_examples 'malicious regexp' do shared_examples 'malicious regexp' do
let(:malicious_text) { 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!' } 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 it 'takes under a second' do
expect { Timeout.timeout(1) { subject } }.not_to raise_error expect { Timeout.timeout(1) { subject } }.not_to raise_error
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment