From 72a57525a87b694799cd6406e8e8f117a902a890 Mon Sep 17 00:00:00 2001 From: Robert Speicher <robert@gitlab.com> Date: Thu, 21 Dec 2017 18:34:34 +0000 Subject: [PATCH] Merge branch 'ac/41346-xss-ci-job-output' into 'security-10-3' [10.3] Fix XSS vulnerability in Pipeline job trace See merge request gitlab/gitlabhq!2258 (cherry picked from commit 44caa80ed9a2514a74a5eeab10ff51849d64851b) 5f86f3ff Fix XSS vulnerability in Pipeline job trace --- lib/gitlab/regex.rb | 2 +- spec/lib/gitlab/ci/ansi2html_spec.rb | 55 ++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 0002c7da8f..7ab85e1c35 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 05e2d94cbd..7549e9941b 100644 --- a/spec/lib/gitlab/ci/ansi2html_spec.rb +++ b/spec/lib/gitlab/ci/ansi2html_spec.rb @@ -217,11 +217,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 -- 2.30.9