Commit 6c3482d1 authored by Yorick Peterse's avatar Yorick Peterse

Merge branch 'security-50334' into 'master'

Fix git clone revealing private repo's presence

See merge request gitlab/gitlabhq!2937
parents 72db8ae2 9d046c87
---
title: Fix git clone revealing private repo's presence
merge_request:
author:
type: security
...@@ -40,7 +40,7 @@ scope(path: '*namespace_id/:project_id', ...@@ -40,7 +40,7 @@ scope(path: '*namespace_id/:project_id',
# /info/refs?service=git-receive-pack, but nothing else. # /info/refs?service=git-receive-pack, but nothing else.
# #
git_http_handshake = lambda do |request| 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.blank? ||
request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/)) request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/))
end end
......
...@@ -2,12 +2,13 @@ ...@@ -2,12 +2,13 @@
module Constraints module Constraints
class ProjectUrlConstrainer class ProjectUrlConstrainer
def matches?(request) def matches?(request, existence_check: true)
namespace_path = request.params[:namespace_id] namespace_path = request.params[:namespace_id]
project_path = request.params[:project_id] || request.params[:id] project_path = request.params[:project_id] || request.params[:id]
full_path = [namespace_path, project_path].join('/') full_path = [namespace_path, project_path].join('/')
return false unless ProjectPathValidator.valid_path?(full_path) 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 # 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 # as cache for further Project.find_by_full_path calls within request
......
...@@ -16,6 +16,10 @@ describe Constraints::ProjectUrlConstrainer do ...@@ -16,6 +16,10 @@ describe Constraints::ProjectUrlConstrainer do
let(:request) { build_request('foo', 'bar') } let(:request) { build_request('foo', 'bar') }
it { expect(subject.matches?(request)).to be_falsey } 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 end
context "project id ending with .git" do context "project id ending with .git" do
......
...@@ -104,6 +104,70 @@ describe 'Git HTTP requests' do ...@@ -104,6 +104,70 @@ describe 'Git HTTP requests' do
end end
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 describe "User with no identities" do
let(:user) { create(:user) } let(:user) { create(:user) }
...@@ -143,6 +207,10 @@ describe 'Git HTTP requests' do ...@@ -143,6 +207,10 @@ describe 'Git HTTP requests' do
expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(response).to have_gitlab_http_status(:unprocessable_entity)
end end
end end
it_behaves_like 'project path without .git suffix' do
let(:project_path) { "#{user.namespace.path}/project.git-project" }
end
end end
end end
...@@ -706,70 +774,8 @@ describe 'Git HTTP requests' do ...@@ -706,70 +774,8 @@ describe 'Git HTTP requests' do
end end
end end
context "when the project path doesn't end in .git" do it_behaves_like 'project path without .git suffix' do
let(:project) { create(:project, :repository, :public, path: 'project.git-project') } let(:project_path) { create(:project, :repository, :public, path: 'project.git-project').full_path }
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
end end
context "retrieving an info/refs file" do 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