Commit bc10afb6 authored by Robert Speicher's avatar Robert Speicher Committed by Robert Speicher

Merge branch 'dm-go-get-xss' into 'security-9-3'

Fix XSS issue in go-get handling

See merge request !2128
parent b39d0c31
---
title: Fix XSS issue in go-get handling
merge_request:
author:
......@@ -3,6 +3,10 @@
module Gitlab
module Middleware
class Go
include ActionView::Helpers::TagHelper
PROJECT_PATH_REGEX = %r{\A(#{Gitlab::PathRegex.full_namespace_route_regex}/#{Gitlab::PathRegex.project_route_regex})/}.freeze
def initialize(app)
@app = app
end
......@@ -10,17 +14,20 @@ module Gitlab
def call(env)
request = Rack::Request.new(env)
if go_request?(request)
render_go_doc(request)
else
@app.call(env)
end
render_go_doc(request) || @app.call(env)
end
private
def render_go_doc(request)
body = go_body(request)
return unless go_request?(request)
path = project_path(request)
return unless path
body = go_body(path)
return unless body
response = Rack::Response.new(body, 200, { 'Content-Type' => 'text/html' })
response.finish
end
......@@ -29,11 +36,13 @@ module Gitlab
request["go-get"].to_i == 1 && request.env["PATH_INFO"].present?
end
def go_body(request)
project_url = URI.join(Gitlab.config.gitlab.url, project_path(request))
def go_body(path)
project_url = URI.join(Gitlab.config.gitlab.url, path)
import_prefix = strip_url(project_url.to_s)
"<!DOCTYPE html><html><head><meta content='#{import_prefix} git #{project_url}.git' name='go-import'></head></html>\n"
meta_tag = tag :meta, name: 'go-import', content: "#{import_prefix} git #{project_url}.git"
head_tag = content_tag :head, meta_tag
content_tag :html, head_tag
end
def strip_url(url)
......@@ -44,6 +53,10 @@ module Gitlab
path_info = request.env["PATH_INFO"]
path_info.sub!(/^\//, '')
project_path_match = "#{path_info}/".match(PROJECT_PATH_REGEX)
return unless project_path_match
path = project_path_match[1]
# Go subpackages may be in the form of `namespace/project/path1/path2/../pathN`.
# In a traditional project with a single namespace, this would denote repo
# `namespace/project` with subpath `path1/path2/../pathN`, but with nested
......@@ -51,7 +64,7 @@ module Gitlab
# `path2/../pathN`, for example.
# We find all potential project paths out of the path segments
path_segments = path_info.split('/')
path_segments = path.split('/')
simple_project_path = path_segments.first(2).join('/')
# If the path is at most 2 segments long, it is a simple `namespace/project` path and we're done
......
......@@ -79,12 +79,28 @@ describe Gitlab::Middleware::Go do
it_behaves_like 'a nested project'
end
context 'with a subpackage that is not a valid project path' do
let(:path) { "#{project.full_path}/---subpackage" }
it_behaves_like 'a nested project'
end
context 'without subpackages' do
let(:path) { project.full_path }
it_behaves_like 'a nested project'
end
end
context 'with a bogus path' do
let(:path) { "http:;url=http:&sol;&sol;www.example.com'http-equiv='refresh'x='?go-get=1" }
it 'skips go-import generation' do
expect(app).to receive(:call).and_return('no-go')
go
end
end
end
def go
......@@ -100,7 +116,7 @@ describe Gitlab::Middleware::Go do
def expect_response_with_path(response, path)
expect(response[0]).to eq(200)
expect(response[1]['Content-Type']).to eq('text/html')
expected_body = "<!DOCTYPE html><html><head><meta content='#{Gitlab.config.gitlab.host}/#{path} git http://#{Gitlab.config.gitlab.host}/#{path}.git' name='go-import'></head></html>\n"
expected_body = %{<html><head><meta name="go-import" content="#{Gitlab.config.gitlab.host}/#{path} git http://#{Gitlab.config.gitlab.host}/#{path}.git" /></head></html>}
expect(response[2].body).to eq([expected_body])
end
end
......
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