Commit e390d86f authored by Stan Hu's avatar Stan Hu Committed by Jose Ivan Vargas

Merge branch 'jej/security-release-2017-08-10' into 'master'

Security release 2017-08-10 patch

See merge request !13477
parent 54ede0b8
---
title: Remove hidden symlinks from project import files
merge_request:
author:
---
title: Disallow Git URLs that include a username or hostname beginning with a non-alphanumeric
character
merge_request:
author:
...@@ -47,12 +47,16 @@ module Gitlab ...@@ -47,12 +47,16 @@ module Gitlab
end end
def remove_symlinks! def remove_symlinks!
Dir["#{@shared.export_path}/**/*"].each do |path| extracted_files.each do |path|
FileUtils.rm(path) if File.lstat(path).symlink? FileUtils.rm(path) if File.lstat(path).symlink?
end end
true true
end end
def extracted_files
Dir.glob("#{@shared.export_path}/**/*", File::FNM_DOTMATCH).reject { |f| f =~ /.*\/\.{1,2}$/ }
end
end end
end end
end end
...@@ -19,6 +19,8 @@ module Gitlab ...@@ -19,6 +19,8 @@ module Gitlab
return false if internal?(uri) return false if internal?(uri)
return true if blocked_port?(uri.port) return true if blocked_port?(uri.port)
return true if blocked_user_or_hostname?(uri.user)
return true if blocked_user_or_hostname?(uri.hostname)
server_ips = Resolv.getaddresses(uri.hostname) server_ips = Resolv.getaddresses(uri.hostname)
return true if (blocked_ips & server_ips).any? return true if (blocked_ips & server_ips).any?
...@@ -37,6 +39,12 @@ module Gitlab ...@@ -37,6 +39,12 @@ module Gitlab
port < 1024 && !VALID_PORTS.include?(port) port < 1024 && !VALID_PORTS.include?(port)
end end
def blocked_user_or_hostname?(value)
return false if value.blank?
value !~ /\A\p{Alnum}/
end
def internal?(uri) def internal?(uri)
internal_web?(uri) || internal_shell?(uri) internal_web?(uri) || internal_shell?(uri)
end end
......
...@@ -5,6 +5,7 @@ describe Gitlab::ImportExport::FileImporter do ...@@ -5,6 +5,7 @@ describe Gitlab::ImportExport::FileImporter do
let(:export_path) { "#{Dir.tmpdir}/file_importer_spec" } let(:export_path) { "#{Dir.tmpdir}/file_importer_spec" }
let(:valid_file) { "#{shared.export_path}/valid.json" } let(:valid_file) { "#{shared.export_path}/valid.json" }
let(:symlink_file) { "#{shared.export_path}/invalid.json" } let(:symlink_file) { "#{shared.export_path}/invalid.json" }
let(:hidden_symlink_file) { "#{shared.export_path}/.hidden" }
let(:subfolder_symlink_file) { "#{shared.export_path}/subfolder/invalid.json" } let(:subfolder_symlink_file) { "#{shared.export_path}/subfolder/invalid.json" }
before do before do
...@@ -25,6 +26,10 @@ describe Gitlab::ImportExport::FileImporter do ...@@ -25,6 +26,10 @@ describe Gitlab::ImportExport::FileImporter do
expect(File.exist?(symlink_file)).to be false expect(File.exist?(symlink_file)).to be false
end end
it 'removes hidden symlinks in root folder' do
expect(File.exist?(hidden_symlink_file)).to be false
end
it 'removes symlinks in subfolders' do it 'removes symlinks in subfolders' do
expect(File.exist?(subfolder_symlink_file)).to be false expect(File.exist?(subfolder_symlink_file)).to be false
end end
......
...@@ -20,6 +20,34 @@ describe Gitlab::UrlBlocker do ...@@ -20,6 +20,34 @@ describe Gitlab::UrlBlocker do
expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true
end end
it 'returns true for a non-alphanumeric hostname' do
stub_resolv
aggregate_failures do
expect(described_class).to be_blocked_url('ssh://-oProxyCommand=whoami/a')
# The leading character here is a Unicode "soft hyphen"
expect(described_class).to be_blocked_url('ssh://­oProxyCommand=whoami/a')
# Unicode alphanumerics are allowed
expect(described_class).not_to be_blocked_url('ssh://ğitlab.com/a')
end
end
it 'returns true for a non-alphanumeric username' do
stub_resolv
aggregate_failures do
expect(described_class).to be_blocked_url('ssh://-oProxyCommand=whoami@example.com/a')
# The leading character here is a Unicode "soft hyphen"
expect(described_class).to be_blocked_url('ssh://­oProxyCommand=whoami@example.com/a')
# Unicode alphanumerics are allowed
expect(described_class).not_to be_blocked_url('ssh://ğitlab@example.com/a')
end
end
it 'returns true for invalid URL' do it 'returns true for invalid URL' do
expect(described_class.blocked_url?('http://:8080')).to be true expect(described_class.blocked_url?('http://:8080')).to be true
end end
...@@ -28,4 +56,10 @@ describe Gitlab::UrlBlocker do ...@@ -28,4 +56,10 @@ describe Gitlab::UrlBlocker do
expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false
end end
end end
# Resolv does not support resolving UTF-8 domain names
# See https://bugs.ruby-lang.org/issues/4270
def stub_resolv
allow(Resolv).to receive(:getaddresses).and_return([])
end
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