Commit ba60d4f6 authored by Douwe Maan's avatar Douwe Maan Committed by Mike Greiling

Merge branch '24570-use-re2-for-user-supplied-regexp-9-3' into 'security-9-3'

24570 use re2 for user supplied regexp 9 3

See merge request !2129
parent ceda6bd5
...@@ -163,6 +163,9 @@ gem 'rainbow', '~> 2.2' ...@@ -163,6 +163,9 @@ gem 'rainbow', '~> 2.2'
# GitLab settings # GitLab settings
gem 'settingslogic', '~> 2.0.9' gem 'settingslogic', '~> 2.0.9'
# Linear-time regex library for untrusted regular expressions
gem 're2', '~> 1.0.0'
# Misc # Misc
gem 'version_sorter', '~> 2.1.0' gem 'version_sorter', '~> 2.1.0'
......
...@@ -657,6 +657,7 @@ GEM ...@@ -657,6 +657,7 @@ GEM
debugger-ruby_core_source (~> 1.3) debugger-ruby_core_source (~> 1.3)
rdoc (4.2.2) rdoc (4.2.2)
json (~> 1.4) json (~> 1.4)
re2 (1.0.0)
recaptcha (3.0.0) recaptcha (3.0.0)
json json
recursive-open-struct (1.0.0) recursive-open-struct (1.0.0)
...@@ -1056,6 +1057,7 @@ DEPENDENCIES ...@@ -1056,6 +1057,7 @@ DEPENDENCIES
raindrops (~> 0.18) raindrops (~> 0.18)
rblineprof (~> 0.3.6) rblineprof (~> 0.3.6)
rdoc (~> 4.2) rdoc (~> 4.2)
re2 (~> 1.0.0)
recaptcha (~> 3.0) recaptcha (~> 3.0)
redcarpet (~> 3.4) redcarpet (~> 3.4)
redis (~> 3.2) redis (~> 3.2)
......
...@@ -69,12 +69,12 @@ module Gitlab ...@@ -69,12 +69,12 @@ module Gitlab
return unless valid? return unless valid?
return unless regex return unless regex
regex = Regexp.new(regex) regex = Gitlab::UntrustedRegexp.new(regex)
match = "" match = ""
reverse_line do |line| reverse_line do |line|
matches = line.scan(regex) matches = regex.scan(line)
next unless matches.is_a?(Array) next unless matches.is_a?(Array)
next if matches.empty? next if matches.empty?
......
...@@ -18,7 +18,11 @@ module Gitlab ...@@ -18,7 +18,11 @@ module Gitlab
mapping = @map.find { |mapping| mapping[:source] === path } mapping = @map.find { |mapping| mapping[:source] === path }
return unless mapping return unless mapping
path.sub(mapping[:source], mapping[:public]) if mapping[:source].is_a?(String)
path.sub(mapping[:source], mapping[:public])
else
mapping[:source].replace(path, mapping[:public])
end
end end
private private
...@@ -35,7 +39,7 @@ module Gitlab ...@@ -35,7 +39,7 @@ module Gitlab
source_pattern = source_pattern[1...-1].gsub('\/', '/') source_pattern = source_pattern[1...-1].gsub('\/', '/')
begin begin
source_pattern = /\A#{source_pattern}\z/ source_pattern = Gitlab::UntrustedRegexp.new('\A' + source_pattern + '\z')
rescue RegexpError => e rescue RegexpError => e
raise FormatError, "Route map entry source is not a valid regular expression: #{e}" raise FormatError, "Route map entry source is not a valid regular expression: #{e}"
end end
......
module Gitlab
# An untrusted regular expression is any regexp containing patterns sourced
# from user input.
#
# Ruby's built-in regular expression library allows patterns which complete in
# exponential time, permitting denial-of-service attacks.
#
# Not all regular expression features are available in untrusted regexes, and
# there is a strict limit on total execution time. See the RE2 documentation
# at https://github.com/google/re2/wiki/Syntax for more details.
class UntrustedRegexp
delegate :===, to: :regexp
def initialize(pattern)
@regexp = RE2::Regexp.new(pattern, log_errors: false)
raise RegexpError.new(regexp.error) unless regexp.ok?
end
def replace_all(text, rewrite)
RE2.GlobalReplace(text, regexp, rewrite)
end
def scan(text)
scan_regexp.scan(text).map do |match|
if regexp.number_of_capturing_groups == 0
match.first
else
match
end
end
end
def replace(text, rewrite)
RE2.Replace(text, regexp, rewrite)
end
private
attr_reader :regexp
# RE2 scan operates differently to Ruby scan when there are no capture
# groups, so work around it
def scan_regexp
@scan_regexp ||=
if regexp.number_of_capturing_groups == 0
RE2::Regexp.new('(' + regexp.source + ')')
else
regexp
end
end
end
end
...@@ -293,5 +293,12 @@ describe Gitlab::Ci::Trace::Stream do ...@@ -293,5 +293,12 @@ describe Gitlab::Ci::Trace::Stream do
it { is_expected.to eq("65") } it { is_expected.to eq("65") }
end end
context 'malicious regexp' do
let(:data) { malicious_text }
let(:regex) { malicious_regexp }
include_examples 'malicious regexp'
end
end end
end end
...@@ -55,6 +55,19 @@ describe Gitlab::RouteMap, lib: true do ...@@ -55,6 +55,19 @@ describe Gitlab::RouteMap, lib: true do
end end
describe '#public_path_for_source_path' do describe '#public_path_for_source_path' do
context 'malicious regexp' do
include_examples 'malicious regexp'
subject do
map = described_class.new(<<-"MAP".strip_heredoc)
- source: '#{malicious_regexp}'
public: '/'
MAP
map.public_path_for_source_path(malicious_text)
end
end
subject do subject do
described_class.new(<<-'MAP'.strip_heredoc) described_class.new(<<-'MAP'.strip_heredoc)
# Team data # Team data
......
require 'spec_helper'
describe Gitlab::UntrustedRegexp do
describe '#initialize' do
subject { described_class.new(pattern) }
context 'invalid regexp' do
let(:pattern) { '[' }
it { expect { subject }.to raise_error(RegexpError) }
end
end
describe '#replace_all' do
it 'replaces all instances of the match in a string' do
result = described_class.new('foo').replace_all('foo bar foo', 'oof')
expect(result).to eq('oof bar oof')
end
end
describe '#replace' do
it 'replaces the first instance of the match in a string' do
result = described_class.new('foo').replace('foo bar foo', 'oof')
expect(result).to eq('oof bar foo')
end
end
describe '#===' do
it 'returns true for a match' do
result = described_class.new('foo') === 'a foo here'
expect(result).to be_truthy
end
it 'returns false for no match' do
result = described_class.new('foo') === 'a bar here'
expect(result).to be_falsy
end
end
describe '#scan' do
subject { described_class.new(regexp).scan(text) }
context 'malicious regexp' do
let(:text) { malicious_text }
let(:regexp) { malicious_regexp }
include_examples 'malicious regexp'
end
context 'no capture group' do
let(:regexp) { '.+' }
let(:text) { 'foo' }
it 'returns the whole match' do
is_expected.to eq(['foo'])
end
end
context 'one capture group' do
let(:regexp) { '(f).+' }
let(:text) { 'foo' }
it 'returns the captured part' do
is_expected.to eq([%w[f]])
end
end
context 'two capture groups' do
let(:regexp) { '(f).(o)' }
let(:text) { 'foo' }
it 'returns the captured parts' do
is_expected.to eq([%w[f o]])
end
end
end
end
shared_examples 'malicious regexp' do
let(:malicious_text) { 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!' }
let(:malicious_regexp) { '(?i)^(([a-z])+.)+[A-Z]([a-z])+$' }
it 'takes under a second' do
expect { Timeout.timeout(1) { subject } }.not_to raise_error
end
end
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