Commit dbcf037b authored by Mark Chao's avatar Mark Chao
Browse files

Fix git clone revealing private repo's presence

Ensure redirection to path with .git suffix regardless whether project
exists or not.
parent 0d5bd8d7
---
title: Fix git clone revealing private repo's presence
merge_request:
author:
type: security
......@@ -40,7 +40,7 @@
# /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
......
......@@ -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
......
......@@ -16,6 +16,10 @@
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
......
......@@ -104,6 +104,70 @@
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: 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: 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: 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 @@
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 @@ def attempt_login(include_password)
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: 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: 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: 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
......
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