diff --git a/lib/open_project/scm/adapters/git.rb b/lib/open_project/scm/adapters/git.rb index cca3094bd25..c10c09ba002 100644 --- a/lib/open_project/scm/adapters/git.rb +++ b/lib/open_project/scm/adapters/git.rb @@ -201,9 +201,14 @@ module OpenProject end def checkout_path - Pathname(OpenProject::Configuration.scm_local_checkout_path) - .join(@identifier) - .expand_path + root = Pathname(OpenProject::Configuration.scm_local_checkout_path).expand_path.to_s + path = Pathname(root).join(@identifier).expand_path.to_s + + unless path.start_with?("#{root}/") + raise ArgumentError, "Checkout path escapes the configured root directory" + end + + Pathname(path) end def checkout_uri diff --git a/lib/open_project/scm/manageable_repository.rb b/lib/open_project/scm/manageable_repository.rb index 4ed2ce65301..f5cc07db129 100644 --- a/lib/open_project/scm/manageable_repository.rb +++ b/lib/open_project/scm/manageable_repository.rb @@ -111,7 +111,14 @@ module OpenProject # Used only in the creation of a repository, at a later point # in time, it is referred to in the root_url def managed_repository_path - File.join(self.class.managed_root, repository_identifier) + root = File.expand_path(self.class.managed_root) + path = File.expand_path(File.join(root, repository_identifier)) + + unless path.start_with?("#{root}/") + raise ArgumentError, "Repository path escapes the configured managed root directory" + end + + path end ## diff --git a/spec/lib/open_project/scm/adapters/git_adapter_spec.rb b/spec/lib/open_project/scm/adapters/git_adapter_spec.rb index 58f3dc40147..083983b2780 100644 --- a/spec/lib/open_project/scm/adapters/git_adapter_spec.rb +++ b/spec/lib/open_project/scm/adapters/git_adapter_spec.rb @@ -550,6 +550,33 @@ RSpec.describe OpenProject::SCM::Adapters::Git do end end + describe "#checkout_path" do + let(:repos_dir) { Dir.mktmpdir } + + before do + allow(OpenProject::Configuration) + .to receive(:scm_local_checkout_path) + .and_return(repos_dir) + end + + after { FileUtils.remove_entry(repos_dir) } + + it "returns a path within the configured root for a normal identifier" do + adapter = described_class.new("file:///some/repo.git", nil, nil, nil, nil, "my-project") + expect(adapter.checkout_path.to_s).to eq(File.join(repos_dir, "my-project")) + end + + it "raises ArgumentError when the identifier contains path traversal" do + adapter = described_class.new("file:///some/repo.git", nil, nil, nil, nil, "../../etc") + expect { adapter.checkout_path }.to raise_error(ArgumentError, /escapes the configured root/) + end + + it "raises ArgumentError for an identifier that resolves to the root itself" do + adapter = described_class.new("file:///some/repo.git", nil, nil, nil, nil, ".") + expect { adapter.checkout_path }.to raise_error(ArgumentError, /escapes the configured root/) + end + end + context "with a local repository" do it_behaves_like "git adapter specs" end diff --git a/spec/models/repository/git_spec.rb b/spec/models/repository/git_spec.rb index 8f8fa0b4548..f855bb1f21b 100644 --- a/spec/models/repository/git_spec.rb +++ b/spec/models/repository/git_spec.rb @@ -125,6 +125,18 @@ RSpec.describe Repository::Git do end end + context "and project with traversal identifier" do + before do + instance.project = project + allow(instance).to receive(:repository_identifier).and_return("../../etc/evil") + end + + it "raises ArgumentError for path traversal" do + expect { instance.managed_repository_path } + .to raise_error(ArgumentError, /escapes the configured managed root/) + end + end + context "and associated project with parent" do let(:parent) { build(:project) } let(:project) { build(:project, parent:) }