From 7135dfc2c4f018667bb2ac5a2e8b8cff15fc4232 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Wed, 22 Apr 2026 10:57:45 +0200 Subject: [PATCH] Add additional validations for path validation for posix on repositories --- lib/open_project/scm/local_path_validator.rb | 43 +++- .../scm/local_path_validator_spec.rb | 214 ++++++++++++++++++ 2 files changed, 250 insertions(+), 7 deletions(-) create mode 100644 spec/lib/open_project/scm/local_path_validator_spec.rb diff --git a/lib/open_project/scm/local_path_validator.rb b/lib/open_project/scm/local_path_validator.rb index 1cf410ae9d9..a1d12ab0f2a 100644 --- a/lib/open_project/scm/local_path_validator.rb +++ b/lib/open_project/scm/local_path_validator.rb @@ -43,15 +43,44 @@ module OpenProject def local_path(value) return if value.blank? - parsed = URI.parse(value) - - if parsed.scheme == "file" - return File.expand_path(parsed.path) + raw_path = begin + parsed = URI.parse(value) + case parsed.scheme&.downcase + when "file" + # Use the path component extracted by the URI library. + # file:////path gives parsed.path = "//path"; we normalise below. + parsed.path + when nil + # Bare absolute path or an authority-relative reference such as + # ///path, which Ruby's URI library sometimes rejects as invalid. + value + end + # Any other scheme (http, https, git, svn, ssh) => nil => not local. + rescue URI::Error + # Malformed URI – treat as a raw string rather than silently ignoring + # it, so that inputs like ///path cannot bypass the check. + value end - return File.expand_path(value) if parsed.scheme.nil? && value.start_with?("/") - rescue URI::Error - return + return if raw_path.blank? + return unless raw_path.start_with?("/") + + # Collapse any run of leading slashes down to exactly one. + # + # POSIX allows "//" at the start of a path to have + # implementation-defined meaning, and three or more leading slashes + # are equivalent to one. In practice File.expand_path preserves "//", + # so "//path" would not match against the "/path"-rooted forbidden + # roots. Normalising here closes the remaining bypass vectors: + # + # ///app/repos/foo (URI::Error rescue path) + # file:////app/repos/foo (parsed.path == "//app/repos/foo") + # //app/repos/foo (double-slash bare path) + normalized = raw_path.sub(/\A\/{2,}/, "/") + + File.expand_path(normalized) + rescue ArgumentError + nil end def forbidden_roots diff --git a/spec/lib/open_project/scm/local_path_validator_spec.rb b/spec/lib/open_project/scm/local_path_validator_spec.rb new file mode 100644 index 00000000000..6d6734133fe --- /dev/null +++ b/spec/lib/open_project/scm/local_path_validator_spec.rb @@ -0,0 +1,214 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe OpenProject::SCM::LocalPathValidator do + let(:checkout_root) { "/srv/openproject/repositories" } + let(:managed_git_root) { "/srv/openproject/git" } + let(:managed_svn_root) { "/srv/openproject/svn" } + + before do + allow(OpenProject::Configuration).to receive(:scm_local_checkout_path).and_return(checkout_root) + allow(Repository::Git).to receive(:managed_root).and_return(managed_git_root) + allow(Repository::Subversion).to receive(:managed_root).and_return(managed_svn_root) + end + + describe ".local_path" do + subject { described_class.local_path(value) } + + context "with a simple absolute path" do + let(:value) { "/srv/openproject/git/myproject.git" } + + it { is_expected.to eq("/srv/openproject/git/myproject.git") } + end + + context "with a standard file:// URL" do + let(:value) { "file:///srv/openproject/git/myproject.git" } + + it { is_expected.to eq("/srv/openproject/git/myproject.git") } + end + + context "with a file:// URL with localhost authority" do + let(:value) { "file://localhost/srv/openproject/git/myproject.git" } + + it { is_expected.to eq("/srv/openproject/git/myproject.git") } + end + + context "with triple-slash bare path (///path)" do + let(:value) { "///srv/openproject/git/myproject.git" } + + it "normalises to a single leading slash" do + expect(subject).to eq("/srv/openproject/git/myproject.git") + end + end + + context "with double-slash bare path (//path)" do + let(:value) { "//srv/openproject/git/myproject.git" } + + it "normalises to a single leading slash" do + expect(subject).to eq("/srv/openproject/git/myproject.git") + end + end + + context "with file URL containing quadruple slashes (file:////path)" do + let(:value) { "file:////srv/openproject/git/myproject.git" } + + it "normalises to a single leading slash" do + expect(subject).to eq("/srv/openproject/git/myproject.git") + end + end + + context "with dot-dot path segments" do + let(:value) { "/srv/openproject/git/../git/myproject.git" } + + it "resolves the traversal" do + expect(subject).to eq("/srv/openproject/git/myproject.git") + end + end + + context "with an http URL" do + let(:value) { "https://github.com/example/repo.git" } + + it { is_expected.to be_nil } + end + + context "with a git protocol URL" do + let(:value) { "git://github.com/example/repo.git" } + + it { is_expected.to be_nil } + end + + context "with a relative path" do + let(:value) { "relative/path/repo.git" } + + it { is_expected.to be_nil } + end + + context "with a blank value" do + let(:value) { "" } + + it { is_expected.to be_nil } + end + end + + describe ".points_to_openproject_directory?" do + subject { described_class.points_to_openproject_directory?(value) } + + shared_examples "flagged as forbidden" do + it { is_expected.to be true } + end + + shared_examples "allowed through" do + it { is_expected.to be false } + end + + context "with plain absolute path inside git managed root" do + let(:value) { "#{managed_git_root}/other-project.git" } + + it_behaves_like "flagged as forbidden" + end + + context "with plain absolute path inside checkout root" do + let(:value) { "#{checkout_root}/other-project" } + + it_behaves_like "flagged as forbidden" + end + + context "with plain absolute path inside svn managed root" do + let(:value) { "#{managed_svn_root}/other-project" } + + it_behaves_like "flagged as forbidden" + end + + context "with standard file:// URL inside managed root" do + let(:value) { "file://#{managed_git_root}/other-project.git" } + + it_behaves_like "flagged as forbidden" + end + + # Bypass A: triple-slash bare path (URI::Error rescue path) + context "with triple-slash path (///path) inside managed root" do + let(:value) { "///#{managed_git_root.delete_prefix('/')}/other-project.git" } + + it_behaves_like "flagged as forbidden" + end + + # Bypass B: double-slash bare path (// survives File.expand_path on POSIX) + context "with double-slash path (//path) inside managed root" do + let(:value) { "//#{managed_git_root.delete_prefix('/')}/other-project.git" } + + it_behaves_like "flagged as forbidden" + end + + # Bypass B/C: file URL with quadruple slashes (parsed.path starts with //) + context "with file URL with quadruple slashes (file:////path) inside managed root" do + let(:value) { "file:////#{managed_git_root.delete_prefix('/')}/other-project.git" } + + it_behaves_like "flagged as forbidden" + end + + context "with file URL with localhost authority inside managed root" do + let(:value) { "file://localhost#{managed_git_root}/other-project.git" } + + it_behaves_like "flagged as forbidden" + end + + context "with path with dot-dot traversal resolving into managed root" do + let(:value) { "/external/../#{managed_git_root.delete_prefix('/')}/other-project.git" } + + it_behaves_like "flagged as forbidden" + end + + context "with external bare git repository" do + let(:value) { "/srv/external/repos/myrepo.git" } + + it_behaves_like "allowed through" + end + + context "with https remote URL" do + let(:value) { "https://github.com/example/repo.git" } + + it_behaves_like "allowed through" + end + + context "with path that only shares a prefix with managed root" do + let(:value) { "#{managed_git_root}-extra/repo.git" } + + it_behaves_like "allowed through" + end + + context "when blank value" do + let(:value) { nil } + + it_behaves_like "allowed through" + end + end +end