Escape title attributes in references

13 files changed, 52 insertions, 7 deletions

diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 7b034f22248..c3b4731dff3 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -109,5 +109,6 @@ module IssuesHelper
     end
   end
 
+  # Required for Gitlab::Markdown::IssueReferenceFilter
   module_function :url_for_issue, :title_for_issue
 end
diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb
index 0259829a059..8272c177d59 100644
--- a/app/helpers/labels_helper.rb
+++ b/app/helpers/labels_helper.rb
@@ -1,4 +1,6 @@
 module LabelsHelper
+  include ActionView::Helpers::TagHelper
+
   def project_label_names
     @project.labels.pluck(:title)
   end
@@ -11,7 +13,7 @@ module LabelsHelper
     # by LabelReferenceFilter
     span = %(<span class="label color-label") +
       %( style="background-color: #{label_color}; color: #{text_color}">) +
-      label.name + '</span>'
+      escape_once(label.name) + '</span>'
 
     span.html_safe
   end
@@ -56,5 +58,6 @@ module LabelsHelper
     options_from_collection_for_select(project.labels, 'name', 'name', params[:label_name])
   end
 
-  module_function :render_colored_label, :text_color_for_bg
+  # Required for Gitlab::Markdown::LabelReferenceFilter
+  module_function :render_colored_label, :text_color_for_bg, :escape_once
 end
diff --git a/lib/gitlab/markdown/commit_reference_filter.rb b/lib/gitlab/markdown/commit_reference_filter.rb
index 914eb29dc0c..5a7eca7e5b0 100644
--- a/lib/gitlab/markdown/commit_reference_filter.rb
+++ b/lib/gitlab/markdown/commit_reference_filter.rb
@@ -50,7 +50,7 @@ module Gitlab
           if project.valid_repo? && commit = project.repository.commit(commit_ref)
             url = url_for_commit(project, commit)
 
-            title = commit.link_title
+            title = escape_once(commit.link_title)
             klass = reference_class(:commit)
 
             project_ref += '@' if project_ref
diff --git a/lib/gitlab/markdown/external_issue_reference_filter.rb b/lib/gitlab/markdown/external_issue_reference_filter.rb
index cbbadc79847..0fc3f4cca06 100644
--- a/lib/gitlab/markdown/external_issue_reference_filter.rb
+++ b/lib/gitlab/markdown/external_issue_reference_filter.rb
@@ -46,7 +46,7 @@ module Gitlab
         self.class.references_in(text) do |match, issue|
           url = url_for_issue(issue, project, only_path: context[:only_path])
 
-          title = "Issue in #{project.external_issue_tracker.title}"
+          title = escape_once("Issue in #{project.external_issue_tracker.title}")
           klass = reference_class(:issue)
 
           %(<a href="#{url}"
diff --git a/lib/gitlab/markdown/issue_reference_filter.rb b/lib/gitlab/markdown/issue_reference_filter.rb
index 680daaf6a1d..13d2ba4bab3 100644
--- a/lib/gitlab/markdown/issue_reference_filter.rb
+++ b/lib/gitlab/markdown/issue_reference_filter.rb
@@ -50,7 +50,7 @@ module Gitlab
           if project.issue_exists?(issue)
             url = url_for_issue(issue, project, only_path: context[:only_path])
 
-            title = "Issue: #{title_for_issue(issue, project)}"
+            title = escape_once("Issue: #{title_for_issue(issue, project)}")
             klass = reference_class(:issue)
 
             %(<a href="#{url}"
diff --git a/lib/gitlab/markdown/merge_request_reference_filter.rb b/lib/gitlab/markdown/merge_request_reference_filter.rb
index 15f0c09ab00..372543783e6 100644
--- a/lib/gitlab/markdown/merge_request_reference_filter.rb
+++ b/lib/gitlab/markdown/merge_request_reference_filter.rb
@@ -52,7 +52,7 @@ module Gitlab
           project = self.project_from_ref(project_ref)
 
           if merge_request = project.merge_requests.find_by(iid: id)
-            title = "Merge Request: #{merge_request.title}"
+            title = escape_once("Merge Request: #{merge_request.title}")
             klass = reference_class(:merge_request)
 
             url = url_for_merge_request(merge_request, project)
diff --git a/lib/gitlab/markdown/reference_filter.rb b/lib/gitlab/markdown/reference_filter.rb
index 7bd14020ecc..26663c8d990 100644
--- a/lib/gitlab/markdown/reference_filter.rb
+++ b/lib/gitlab/markdown/reference_filter.rb
@@ -1,3 +1,4 @@
+require 'active_support/core_ext/string/output_safety'
 require 'html/pipeline'
 
 module Gitlab
@@ -12,6 +13,10 @@ module Gitlab
     #   :only_path          - Generate path-only links.
     #
     class ReferenceFilter < HTML::Pipeline::Filter
+      def escape_once(html)
+        ERB::Util.html_escape_once(html)
+      end
+
       # Don't look for references in text nodes that are children of these
       # elements.
       IGNORE_PARENTS = %w(pre code a style).to_set
diff --git a/lib/gitlab/markdown/snippet_reference_filter.rb b/lib/gitlab/markdown/snippet_reference_filter.rb
index 193a548af92..9cada5abaa0 100644
--- a/lib/gitlab/markdown/snippet_reference_filter.rb
+++ b/lib/gitlab/markdown/snippet_reference_filter.rb
@@ -48,7 +48,7 @@ module Gitlab
           project = self.project_from_ref(project_ref)
 
           if snippet = project.snippets.find_by(id: id)
-            title = "Snippet: #{snippet.title}"
+            title = escape_once("Snippet: #{snippet.title}")
             klass = reference_class(:snippet)
 
             url = url_for_snippet(snippet, project)
diff --git a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb
index 84f773669f2..55256fa3a90 100644
--- a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb
@@ -51,6 +51,13 @@ module Gitlab::Markdown
         expect(doc.css('a').first.attr('title')).to eq commit.link_title
       end
 
+      it 'escapes the title attribute' do
+        allow_any_instance_of(Commit).to receive(:title).and_return(%{"></a>whatever<a title="})
+
+        doc = filter("See #{reference}")
+        expect(doc.text).to eq "See #{commit.id}"
+      end
+
       it 'includes default classes' do
         doc = filter("See #{reference}")
         expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-commit'
diff --git a/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb b/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb
index 37c91195202..27e930ef7da 100644
--- a/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb
@@ -80,6 +80,14 @@ module Gitlab::Markdown
         expect(doc.css('a').first.attr('title')).to eq "Issue in JIRA tracker"
       end
 
+      it 'escapes the title attribute' do
+        allow(project.external_issue_tracker).to receive(:title).
+          and_return(%{"></a>whatever<a title="})
+
+        doc = filter("Issue #{reference}")
+        expect(doc.text).to eq "Issue #{reference}"
+      end
+
       it 'includes default classes' do
         doc = filter("Issue #{reference}")
         expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-issue'
diff --git a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb
index 8cbf00a3de2..892e530527e 100644
--- a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb
@@ -57,6 +57,13 @@ module Gitlab::Markdown
         expect(doc.css('a').first.attr('title')).to eq "Issue: #{issue.title}"
       end
 
+      it 'escapes the title attribute' do
+        issue.update_attribute(:title, %{"></a>whatever<a title="})
+
+        doc = filter("Issue #{reference}")
+        expect(doc.text).to eq "Issue #{reference}"
+      end
+
       it 'includes default classes' do
         doc = filter("Issue #{reference}")
         expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-issue'
diff --git a/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb b/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb
index c3026edbf89..c9be5d5deae 100644
--- a/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb
@@ -45,6 +45,13 @@ module Gitlab::Markdown
         expect(doc.css('a').first.attr('title')).to eq "Merge Request: #{merge.title}"
       end
 
+      it 'escapes the title attribute' do
+        merge.update_attribute(:title, %{"></a>whatever<a title="})
+
+        doc = filter("Merge #{reference}")
+        expect(doc.text).to eq "Merge #{reference}"
+      end
+
       it 'includes default classes' do
         doc = filter("Merge #{reference}")
         expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-merge_request'
diff --git a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb
index 3d30cacb0b1..285d6dc8547 100644
--- a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb
@@ -44,6 +44,13 @@ module Gitlab::Markdown
         expect(doc.css('a').first.attr('title')).to eq "Snippet: #{snippet.title}"
       end
 
+      it 'escapes the title attribute' do
+        snippet.update_attribute(:title, %{"></a>whatever<a title="})
+
+        doc = filter("Snippet #{reference}")
+        expect(doc.text).to eq "Snippet #{reference}"
+      end
+
       it 'includes default classes' do
         doc = filter("Snippet #{reference}")
         expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-snippet'