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

2 files changed, 52 insertions, 5 deletions

diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 0002c7da8f1..7ab85e1c35c 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 05e2d94cbd6..7549e9941b6 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('<', '&lt;')
 
-      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