diff options
author | Robert Speicher <robert@gitlab.com> | 2017-12-22 15:05:14 +0000 |
---|---|---|
committer | Tiago Botelho <tiago@gitlab.com> | 2018-01-08 13:30:05 +0000 |
commit | 6ae148195c61378e1f43f18216205821de1fd2af (patch) | |
tree | 41a915eaa92a9aba4d6bedbff957dc48f0a5dd74 | |
parent | 43100ddc56ee88c294588b6c60523a7bc0aebaa9 (diff) | |
download | gitlab-ce-6ae148195c61378e1f43f18216205821de1fd2af.tar.gz |
Merge branch 'ac/41346-xss-ci-job-output-backport-10-2' into 'security-10-2'
[10.2] Fix XSS vulnerability in Pipeline job trace - backport 10 2
See merge request gitlab/gitlabhq!2260
(cherry picked from commit 4ba826b5df561e85f6fdfc86c20779b1a91b598b)
b890d809 Fix XSS vulnerability in Pipeline job trace
-rw-r--r-- | changelogs/unreleased/ac-41346-xss-ci-job-output.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/regex.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/ansi2html_spec.rb | 55 |
3 files changed, 57 insertions, 5 deletions
diff --git a/changelogs/unreleased/ac-41346-xss-ci-job-output.yml b/changelogs/unreleased/ac-41346-xss-ci-job-output.yml new file mode 100644 index 00000000000..5ef42e49b71 --- /dev/null +++ b/changelogs/unreleased/ac-41346-xss-ci-job-output.yml @@ -0,0 +1,5 @@ +--- +title: Fix XSS vulnerability in pipeline job trace +merge_request: +author: +type: security diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 2c7b8af83f2..5065be33f64 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -67,7 +67,7 @@ module Gitlab end def build_trace_section_regex - @build_trace_section_regexp ||= /section_((?:start)|(?:end)):(\d+):([^\r]+)\r\033\[0K/.freeze + @build_trace_section_regexp ||= /section_((?:start)|(?:end)):(\d+):([a-zA-Z0-9_.-]+)\r\033\[0K/.freeze end end end diff --git a/spec/lib/gitlab/ci/ansi2html_spec.rb b/spec/lib/gitlab/ci/ansi2html_spec.rb index 33540eab5d6..d8df1312c47 100644 --- a/spec/lib/gitlab/ci/ansi2html_spec.rb +++ b/spec/lib/gitlab/ci/ansi2html_spec.rb @@ -213,11 +213,58 @@ describe Gitlab::Ci::Ansi2html do "#{section_end[0...-5]}</div>" end - it "prints light red" do - text = "#{section_start}\e[91mHello\e[0m\n#{section_end}" - html = %{#{section_start_html}<span class="term-fg-l-red">Hello</span><br>#{section_end_html}} + shared_examples 'forbidden char in section_name' do + it 'ignores sections' do + text = "#{section_start}Some text#{section_end}" + html = text.gsub("\033[0K", '').gsub('<', '<') - expect(convert_html(text)).to eq(html) + expect(convert_html(text)).to eq(html) + end + end + + shared_examples 'a legit section' do + let(:text) { "#{section_start}Some text#{section_end}" } + + it 'prints light red' do + text = "#{section_start}\e[91mHello\e[0m\n#{section_end}" + html = %{#{section_start_html}<span class="term-fg-l-red">Hello</span><br>#{section_end_html}} + + expect(convert_html(text)).to eq(html) + end + + it 'begins with a section_start html marker' do + expect(convert_html(text)).to start_with(section_start_html) + end + + it 'ends with a section_end html marker' do + expect(convert_html(text)).to end_with(section_end_html) + end + end + + it_behaves_like 'a legit section' + + context 'section name includes $' do + let(:section_name) { 'my_$ection'} + + it_behaves_like 'forbidden char in section_name' + end + + context 'section name includes <' do + let(:section_name) { '<a_tag>'} + + it_behaves_like 'forbidden char in section_name' + end + + context 'section name contains .-_' do + let(:section_name) { 'a.Legit-SeCtIoN_namE' } + + it_behaves_like 'a legit section' + end + + it 'do not allow XSS injections' do + text = "#{section_start}section_end:1:2<script>alert('XSS Hack!');</script>#{section_end}" + + expect(convert_html(text)).not_to include('<script>') end end |