From df41148662142ce20a77b092665f48dd4dfa7bfb Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 2 Jan 2016 20:09:21 +0100
Subject: [PATCH] Improve path sanitization in `StringPath`
---
app/models/ci/build.rb | 10 +++++-----
lib/gitlab/string_path.rb | 17 ++++++++---------
spec/lib/gitlab/string_path_spec.rb | 1 -
3 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index f6783e21d90..df51a5ce079 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -352,15 +352,15 @@ module Ci
def artifacts_metadata_for_path(path)
return [] unless artifacts_metadata.exists?
paths, metadata = [], []
- meta_path = path.sub(/^\.\//, '')
+ metadata_path = path.sub(/^\.\//, '')
File.open(artifacts_metadata.path) do |file|
gzip = Zlib::GzipReader.new(file)
gzip.each_line do |line|
- if line =~ %r{^#{meta_path}[^/]+/?\s}
- path, meta = line.split(' ')
- paths << path
- metadata << JSON.parse(meta)
+ if line =~ %r{^#{Regexp.escape(metadata_path)}[^/\s]+/?\s}
+ matched_path, matched_meta = line.split(' ')
+ paths << matched_path
+ metadata << JSON.parse(matched_meta)
end
end
gzip.close
diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb
index 8310564646e..e7d99a35869 100644
--- a/lib/gitlab/string_path.rb
+++ b/lib/gitlab/string_path.rb
@@ -57,7 +57,7 @@ module Gitlab
def descendants
return [] unless directory?
- select { |entry| entry =~ /^#{@path}.+/ }
+ select { |entry| entry =~ /^#{Regexp.escape(@path)}.+/ }
end
def children
@@ -65,7 +65,7 @@ module Gitlab
return @children if @children
@children = select do |entry|
- self.class.child?(@path, entry)
+ entry =~ %r{^#{Regexp.escape(@path)}[^/\s]+/?$}
end
end
@@ -75,7 +75,7 @@ module Gitlab
end
def directories!
- has_parent? ? directories.prepend(new(@path + '../')) : directories
+ has_parent? ? directories.prepend(parent) : directories
end
def files
@@ -115,13 +115,12 @@ module Gitlab
# It looks like Pathname#new doesn't touch a file system,
# neither Pathname#cleanpath does, so it is, hopefully, filesystem safe
- clean = Pathname.new(path).cleanpath.to_s
- raise ArgumentError, 'Invalid path' if clean.start_with?('../')
- clean + (path.end_with?('/') ? '/' : '')
- end
+ clean_path = Pathname.new(path).cleanpath.to_s
+ raise ArgumentError, 'Invalid path' if clean_path.start_with?('../')
- def self.child?(path, entry)
- entry =~ %r{^#{path}[^/\s]+/?$}
+ prefix = './' unless clean_path =~ %r{^[\.|/]}
+ suffix = '/' if path.end_with?('/') || clean_path =~ /^[\.|\.\.]$/
+ prefix.to_s + clean_path + suffix.to_s
end
end
end
diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb
index af46f9754ac..0ef2155cb9b 100644
--- a/spec/lib/gitlab/string_path_spec.rb
+++ b/spec/lib/gitlab/string_path_spec.rb
@@ -45,7 +45,6 @@ describe Gitlab::StringPath do
it { is_expected.to be_directory }
it { is_expected.to be_relative }
- it { is_expected.to have_parent }
end
describe 'path/dir_1/', path: 'path/dir_1/' do
--
GitLab