diff options
| author | Robert Speicher <robert@gitlab.com> | 2017-06-29 01:22:22 +0000 |
|---|---|---|
| committer | Robert Speicher <rspeicher@gmail.com> | 2017-09-07 20:22:16 -0400 |
| commit | bc10afb600a8079fe250e7c82bf16763a8fed28f (patch) | |
| tree | 6a517e4ff3f1be720fb4de6750a4203724491219 | |
| parent | b39d0c318921bae2e3a11df9ee6828291dad9864 (diff) | |
| download | gitlab-ce-bc10afb600a8079fe250e7c82bf16763a8fed28f.tar.gz | |
Merge branch 'dm-go-get-xss' into 'security-9-3'
Fix XSS issue in go-get handling
See merge request !2128
| -rw-r--r-- | changelogs/unreleased/dm-go-get-xss.yml | 4 | ||||
| -rw-r--r-- | lib/gitlab/middleware/go.rb | 33 | ||||
| -rw-r--r-- | spec/lib/gitlab/middleware/go_spec.rb | 18 |
3 files changed, 44 insertions, 11 deletions
diff --git a/changelogs/unreleased/dm-go-get-xss.yml b/changelogs/unreleased/dm-go-get-xss.yml new file mode 100644 index 00000000000..bf0a7f4bd3d --- /dev/null +++ b/changelogs/unreleased/dm-go-get-xss.yml @@ -0,0 +1,4 @@ +--- +title: Fix XSS issue in go-get handling +merge_request: +author: diff --git a/lib/gitlab/middleware/go.rb b/lib/gitlab/middleware/go.rb index 6023fa1820f..f42168c720e 100644 --- a/lib/gitlab/middleware/go.rb +++ b/lib/gitlab/middleware/go.rb @@ -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 diff --git a/spec/lib/gitlab/middleware/go_spec.rb b/spec/lib/gitlab/middleware/go_spec.rb index 6af1564da19..cab662819ac 100644 --- a/spec/lib/gitlab/middleware/go_spec.rb +++ b/spec/lib/gitlab/middleware/go_spec.rb @@ -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://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 |