Commit f9ef74fc authored by Yorick Peterse's avatar Yorick Peterse

Merge branch 'security-2770-verify-bundle-import-files-11-5' into 'security-11-5'

[11.5] Validate bundle files before unpacking them

See merge request gitlab/gitlabhq!2775

(cherry picked from commit 28bec61b5d3c43ef896780cb0eebf09353b51995)

68433868 Validate bundle files before unpacking them
parent a01a29e0
title: Validate bundle files before unpacking them
type: security
# frozen_string_literal: true
module Gitlab
module Git
class BundleFile
# All git bundle files start with this string
MAGIC = "# v2 git bundle\n"
InvalidBundleError =
attr_reader :filename
def self.check!(filename)
def initialize(filename)
@filename = filename
def check!
data =, 'r') { |f| }
raise InvalidBundleError, 'Invalid bundle file' unless data == MAGIC
...@@ -770,6 +770,11 @@ module Gitlab ...@@ -770,6 +770,11 @@ module Gitlab
end end
def create_from_bundle(bundle_path) def create_from_bundle(bundle_path)
# It's important to check that the linked-to file is actually a valid
# .bundle file as it is passed to `git clone`, which may otherwise
# interpret it as a pointer to another repository
gitaly_repository_client.create_from_bundle(bundle_path) gitaly_repository_client.create_from_bundle(bundle_path)
end end
require 'spec_helper'
describe Gitlab::Git::BundleFile do
describe '.check!' do
let(:valid_bundle) { }
let(:valid_bundle_path) { valid_bundle.path }
let(:invalid_bundle_path) { Rails.root.join('spec/fixtures/malicious.bundle') }
after do
it 'returns nil for a valid bundle' do
valid_bundle.write("# v2 git bundle\nfoo bar baz\n")
expect(described_class.check!(valid_bundle_path)).to be_nil
it 'raises an exception for an invalid bundle' do
expect do
described_class.check!(invalid_bundle_path) raise_error(described_class::InvalidBundleError)
...@@ -1726,22 +1726,23 @@ describe Gitlab::Git::Repository, :seed_helper do ...@@ -1726,22 +1726,23 @@ describe Gitlab::Git::Repository, :seed_helper do
end end
describe '#create_from_bundle' do describe '#create_from_bundle' do
let(:bundle_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") } let(:valid_bundle_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") }
let(:malicious_bundle_path) { Rails.root.join('spec/fixtures/malicious.bundle') }
let(:project) { create(:project) } let(:project) { create(:project) }
let(:imported_repo) { project.repository.raw } let(:imported_repo) { project.repository.raw }
before do before do
expect(repository.bundle_to_disk(bundle_path)).to be_truthy expect(repository.bundle_to_disk(valid_bundle_path)).to be_truthy
end end
after do after do
FileUtils.rm_rf(bundle_path) FileUtils.rm_rf(valid_bundle_path)
end end
it 'creates a repo from a bundle file' do it 'creates a repo from a bundle file' do
expect(imported_repo).not_to exist expect(imported_repo).not_to exist
result = imported_repo.create_from_bundle(bundle_path) result = imported_repo.create_from_bundle(valid_bundle_path)
expect(result).to be_truthy expect(result).to be_truthy
expect(imported_repo).to exist expect(imported_repo).to exist
...@@ -1749,11 +1750,17 @@ describe Gitlab::Git::Repository, :seed_helper do ...@@ -1749,11 +1750,17 @@ describe Gitlab::Git::Repository, :seed_helper do
end end
it 'creates a symlink to the global hooks dir' do it 'creates a symlink to the global hooks dir' do
imported_repo.create_from_bundle(bundle_path) imported_repo.create_from_bundle(valid_bundle_path)
hooks_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { File.join(imported_repo.path, 'hooks') } hooks_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { File.join(imported_repo.path, 'hooks') }
expect(File.readlink(hooks_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) expect(File.readlink(hooks_path)).to eq(Gitlab.config.gitlab_shell.hooks_path)
end end
it 'raises an error if the bundle is an attempted malicious payload' do
expect do
imported_repo.create_from_bundle(malicious_bundle_path) raise_error(::Gitlab::Git::BundleFile::InvalidBundleError)
end end
describe '#checksum' do describe '#checksum' do
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