Commit 532a0b60 authored by James Lopez's avatar James Lopez Committed by Stan Hu

Merge branch 'fix/import-rce-10-3' into 'security-10-3'

[10.3] Fix RCE via project import mechanism

See merge request gitlab/gitlabhq!2294

(cherry picked from commit dcfec507d6f9ee119d65a832393e7c593af1d3b2)

86d75812 Fix RCE via project import mechanism
parent 791ca43f
---
title: Fix RCE via project import mechanism
merge_request:
author:
type: security
...@@ -17,12 +17,16 @@ module Gitlab ...@@ -17,12 +17,16 @@ module Gitlab
def import def import
mkdir_p(@shared.export_path) mkdir_p(@shared.export_path)
remove_symlinks!
wait_for_archived_file do wait_for_archived_file do
decompress_archive decompress_archive
end end
rescue => e rescue => e
@shared.error(e) @shared.error(e)
false false
ensure
remove_symlinks!
end end
private private
...@@ -43,7 +47,7 @@ module Gitlab ...@@ -43,7 +47,7 @@ module Gitlab
raise Projects::ImportService::Error.new("Unable to decompress #{@archive_file} into #{@shared.export_path}") unless result raise Projects::ImportService::Error.new("Unable to decompress #{@archive_file} into #{@shared.export_path}") unless result
remove_symlinks! result
end end
def remove_symlinks! def remove_symlinks!
......
...@@ -37,7 +37,7 @@ module Gitlab ...@@ -37,7 +37,7 @@ module Gitlab
end end
def archive_file def archive_file
@archive_file ||= File.join(@shared.export_path, '..', Gitlab::ImportExport.export_filename(project: @project)) @archive_file ||= File.join(@shared.archive_path, Gitlab::ImportExport.export_filename(project: @project))
end end
end end
end end
......
...@@ -9,7 +9,11 @@ module Gitlab ...@@ -9,7 +9,11 @@ module Gitlab
end end
def export_path def export_path
@export_path ||= Gitlab::ImportExport.export_path(relative_path: opts[:relative_path]) @export_path ||= Gitlab::ImportExport.export_path(relative_path: relative_path)
end
def archive_path
@archive_path ||= Gitlab::ImportExport.export_path(relative_path: relative_archive_path)
end end
def error(error) def error(error)
...@@ -21,6 +25,14 @@ module Gitlab ...@@ -21,6 +25,14 @@ module Gitlab
private private
def relative_path
File.join(opts[:relative_path], SecureRandom.hex)
end
def relative_archive_path
File.join(opts[:relative_path], '..')
end
def error_out(message, caller) def error_out(message, caller)
Rails.logger.error("Import/Export error raised on #{caller}: #{message}") Rails.logger.error("Import/Export error raised on #{caller}: #{message}")
end end
......
...@@ -12,16 +12,19 @@ describe Gitlab::ImportExport::FileImporter do ...@@ -12,16 +12,19 @@ describe Gitlab::ImportExport::FileImporter do
stub_const('Gitlab::ImportExport::FileImporter::MAX_RETRIES', 0) stub_const('Gitlab::ImportExport::FileImporter::MAX_RETRIES', 0)
allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
allow_any_instance_of(Gitlab::ImportExport::CommandLineUtil).to receive(:untar_zxf).and_return(true) allow_any_instance_of(Gitlab::ImportExport::CommandLineUtil).to receive(:untar_zxf).and_return(true)
allow(SecureRandom).to receive(:hex).and_return('abcd')
setup_files setup_files
described_class.import(archive_file: '', shared: shared)
end end
after do after do
FileUtils.rm_rf(export_path) FileUtils.rm_rf(export_path)
end end
context 'normal run' do
before do
described_class.import(archive_file: '', shared: shared)
end
it 'removes symlinks in root folder' do it 'removes symlinks in root folder' do
expect(File.exist?(symlink_file)).to be false expect(File.exist?(symlink_file)).to be false
end end
...@@ -38,6 +41,34 @@ describe Gitlab::ImportExport::FileImporter do ...@@ -38,6 +41,34 @@ describe Gitlab::ImportExport::FileImporter do
expect(File.exist?(valid_file)).to be true expect(File.exist?(valid_file)).to be true
end end
it 'creates the file in the right subfolder' do
expect(shared.export_path).to include('test/abcd')
end
end
context 'error' do
before do
allow_any_instance_of(described_class).to receive(:wait_for_archived_file).and_raise(StandardError)
described_class.import(archive_file: '', shared: shared)
end
it 'removes symlinks in root folder' do
expect(File.exist?(symlink_file)).to be false
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
expect(File.exist?(subfolder_symlink_file)).to be false
end
it 'does not remove a valid file' do
expect(File.exist?(valid_file)).to be true
end
end
def setup_files def setup_files
FileUtils.mkdir_p("#{shared.export_path}/subfolder/") FileUtils.mkdir_p("#{shared.export_path}/subfolder/")
FileUtils.touch(valid_file) FileUtils.touch(valid_file)
......
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