Stop unescaping HTML in BaseLabel#title=, #description=, Timebox#title=

Stop unescaping HTML in BaseLabel#title=, #description=, and Timebox#title=.

This affects anything including BaseLabel or Timebox; namely Label, AntiAbuseReports::Label, Milestone, and Iteration.

Why?

Calling CGI.unescapeHTML immediately after Sanitize.clean is an anti-pattern. Here's an example of Sanitize.clean removing something nasty:

[83] pry(main)> Sanitize.clean("hello <script>alert('xss')</script>")
=> "hello "

What happens if the user enters entities instead?

[84] pry(main)> Sanitize.clean("hello <script>alert('xss')</script>")
=> "hello <script>alert('xss')</script>"

Entities are safe, so this is all OK! But what if we then CGI.unscapeHTML it?

[85] pry(main)> CGI.unescapeHTML(Sanitize.clean("hello <script>alert('xss')</script>"))
=> "hello <script>alert('xss')</script>"

Eep! This is the current state of the application; we are essentially not sanitising the output, because we're allowing the user to enter anything in entities and then unescaping it after sanitisation.

Here's an example demonstrating this, but you can easily test it yourself by going to create a new label through the web interface. SQL logs output by the console have been omitted for clarity.

[96] pry(main)> l = Label.first
=> #<ProjectLabel id:80 gitlab-org/gitlab-test~80>

[99] pry(main)> l.title = "hello &lt;script&gt;alert('xss')&lt;/script&gt;"
=> "hello &lt;script&gt;alert('xss')&lt;/script&gt;"

[100] pry(main)> l.save!
=> true

[101] pry(main)> l.reload
=> #<ProjectLabel id:80 gitlab-org/gitlab-test~80>

[102] pry(main)> l.title
=> "hello <script>alert('xss')</script>"

The good news is, sanitisation is not the answer here! The user should be able to enter any text as a label or milestone title, and that text should be shown exactly as entered --- no sanitisation needed. We must treat it as text, not HTML. Thankfully, we should be doing this already for the most part, seeing as (as mentioned above) we are already allowing users to enter non-sanitised content here via entity unescaping.

Screenshot comparison

In both cases we enter the following label title:

<script>alert(1)</script> &lt;script&gt;alert(1)&lt;/script&gt; &amp;lt;script&amp;gt;alert(1)&amp;lt;/script&amp;gt;

Then we click "Create label", then choose "Edit" from the label's actions.

Before	After

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.