summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--changelogs/unreleased/security-50334.yml5
-rw-r--r--config/routes/git_http.rb2
-rw-r--r--lib/constraints/project_url_constrainer.rb3
-rw-r--r--spec/lib/constraints/project_url_constrainer_spec.rb4
-rw-r--r--spec/requests/git_http_spec.rb134
5 files changed, 82 insertions, 66 deletions
diff --git a/changelogs/unreleased/security-50334.yml b/changelogs/unreleased/security-50334.yml
new file mode 100644
index 00000000000..828ef82b517
--- /dev/null
+++ b/changelogs/unreleased/security-50334.yml
@@ -0,0 +1,5 @@
+---
+title: Fix git clone revealing private repo's presence
+merge_request:
+author:
+type: security
diff --git a/config/routes/git_http.rb b/config/routes/git_http.rb
index ec5c68f81df..a959d40881b 100644
--- a/config/routes/git_http.rb
+++ b/config/routes/git_http.rb
@@ -40,7 +40,7 @@ scope(path: '*namespace_id/:project_id',
# /info/refs?service=git-receive-pack, but nothing else.
#
git_http_handshake = lambda do |request|
- ::Constraints::ProjectUrlConstrainer.new.matches?(request) &&
+ ::Constraints::ProjectUrlConstrainer.new.matches?(request, existence_check: false) &&
(request.query_string.blank? ||
request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/))
end
diff --git a/lib/constraints/project_url_constrainer.rb b/lib/constraints/project_url_constrainer.rb
index eadfbf7bc01..d41490d2ebd 100644
--- a/lib/constraints/project_url_constrainer.rb
+++ b/lib/constraints/project_url_constrainer.rb
@@ -2,12 +2,13 @@
module Constraints
class ProjectUrlConstrainer
- def matches?(request)
+ def matches?(request, existence_check: true)
namespace_path = request.params[:namespace_id]
project_path = request.params[:project_id] || request.params[:id]
full_path = [namespace_path, project_path].join('/')
return false unless ProjectPathValidator.valid_path?(full_path)
+ return true unless existence_check
# We intentionally allow SELECT(*) here so result of this query can be used
# as cache for further Project.find_by_full_path calls within request
diff --git a/spec/lib/constraints/project_url_constrainer_spec.rb b/spec/lib/constraints/project_url_constrainer_spec.rb
index c96e7ab8495..3496b01ebcc 100644
--- a/spec/lib/constraints/project_url_constrainer_spec.rb
+++ b/spec/lib/constraints/project_url_constrainer_spec.rb
@@ -16,6 +16,10 @@ describe Constraints::ProjectUrlConstrainer do
let(:request) { build_request('foo', 'bar') }
it { expect(subject.matches?(request)).to be_falsey }
+
+ context 'existence_check is false' do
+ it { expect(subject.matches?(request, existence_check: false)).to be_truthy }
+ end
end
context "project id ending with .git" do
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 0dc459d9b5a..dd9a9fb7761 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -104,6 +104,70 @@ describe 'Git HTTP requests' do
end
end
+ shared_examples_for 'project path without .git suffix' do
+ context "GET info/refs" do
+ let(:path) { "/#{project_path}/info/refs" }
+
+ context "when no params are added" do
+ before do
+ get path
+ end
+
+ it "redirects to the .git suffix version" do
+ expect(response).to redirect_to("/#{project_path}.git/info/refs")
+ end
+ end
+
+ context "when the upload-pack service is requested" do
+ let(:params) { { service: 'git-upload-pack' } }
+
+ before do
+ get path, params
+ end
+
+ it "redirects to the .git suffix version" do
+ expect(response).to redirect_to("/#{project_path}.git/info/refs?service=#{params[:service]}")
+ end
+ end
+
+ context "when the receive-pack service is requested" do
+ let(:params) { { service: 'git-receive-pack' } }
+
+ before do
+ get path, params
+ end
+
+ it "redirects to the .git suffix version" do
+ expect(response).to redirect_to("/#{project_path}.git/info/refs?service=#{params[:service]}")
+ end
+ end
+
+ context "when the params are anything else" do
+ let(:params) { { service: 'git-implode-pack' } }
+
+ before do
+ get path, params
+ end
+
+ it "redirects to the sign-in page" do
+ expect(response).to redirect_to(new_user_session_path)
+ end
+ end
+ end
+
+ context "POST git-upload-pack" do
+ it "fails to find a route" do
+ expect { clone_post(project_path) }.to raise_error(ActionController::RoutingError)
+ end
+ end
+
+ context "POST git-receive-pack" do
+ it "fails to find a route" do
+ expect { push_post(project_path) }.to raise_error(ActionController::RoutingError)
+ end
+ end
+ end
+
describe "User with no identities" do
let(:user) { create(:user) }
@@ -143,6 +207,10 @@ describe 'Git HTTP requests' do
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
end
+
+ it_behaves_like 'project path without .git suffix' do
+ let(:project_path) { "#{user.namespace.path}/project.git-project" }
+ end
end
end
@@ -706,70 +774,8 @@ describe 'Git HTTP requests' do
end
end
- context "when the project path doesn't end in .git" do
- let(:project) { create(:project, :repository, :public, path: 'project.git-project') }
-
- context "GET info/refs" do
- let(:path) { "/#{project.full_path}/info/refs" }
-
- context "when no params are added" do
- before do
- get path
- end
-
- it "redirects to the .git suffix version" do
- expect(response).to redirect_to("/#{project.full_path}.git/info/refs")
- end
- end
-
- context "when the upload-pack service is requested" do
- let(:params) { { service: 'git-upload-pack' } }
-
- before do
- get path, params
- end
-
- it "redirects to the .git suffix version" do
- expect(response).to redirect_to("/#{project.full_path}.git/info/refs?service=#{params[:service]}")
- end
- end
-
- context "when the receive-pack service is requested" do
- let(:params) { { service: 'git-receive-pack' } }
-
- before do
- get path, params
- end
-
- it "redirects to the .git suffix version" do
- expect(response).to redirect_to("/#{project.full_path}.git/info/refs?service=#{params[:service]}")
- end
- end
-
- context "when the params are anything else" do
- let(:params) { { service: 'git-implode-pack' } }
-
- before do
- get path, params
- end
-
- it "redirects to the sign-in page" do
- expect(response).to redirect_to(new_user_session_path)
- end
- end
- end
-
- context "POST git-upload-pack" do
- it "fails to find a route" do
- expect { clone_post(project.full_path) }.to raise_error(ActionController::RoutingError)
- end
- end
-
- context "POST git-receive-pack" do
- it "fails to find a route" do
- expect { push_post(project.full_path) }.to raise_error(ActionController::RoutingError)
- end
- end
+ it_behaves_like 'project path without .git suffix' do
+ let(:project_path) { create(:project, :repository, :public, path: 'project.git-project').full_path }
end
context "retrieving an info/refs file" do
View Lorry Controller Status