Why is my view being flagged as an XSS vulnerability?

Question

I have a show route that displays the contents of my article

Controller:

def show
  @article = Article.find(params[:id])
end

View:

...
<li class="content"><%= @article.content.html_safe %></li>
...

When running Brakeman, it flags the above as a potential Cross-Site Scripting (XSS) vulnerability

Unescaped model attribute near line 34: Article.find(params[:article_id]).content

I'm trying to figure out what XSS really is and what makes this vulnerable? If someone injected some malicious text or input into the params[:id] field in the route (e.g. /articles/BAD_INPUT) then Article.find() would not find the article and raise an error

The only way the view renders is if a valid Article record is found, right? How else can the user manipulate this?

Thanks!

Edit: I should definitely protect agains the case when an Article is not found and an error is raised, but I figured that's more of a bad design rather than a security vulnerability

Answer 1

Brakeman is warning because the code is taking information from the database and outputting it in a view without escaping it. By default, Brakeman treats values from the database as potentially dangerous. In this case, you probably know the article content is intended to be HTML and is safe to output without escaping it. If you wish to not warn about XSS with values from the database, you can use the --ignore-model-output option.

(The issue you linked in your answer is not really related. Brakeman is expected to warn about uses of raw/html_safe with potentially dangerous values.)

Answer 2

Ok found the answer after some digging.

It apparently has to do with html_safe and raw (which is just an alias for html_safe). The issue is specific to Brakeman and outlined here

That thread says the issue is acknowledged and solved, but it still didn't work for me using the latest version.

I solved it as follows

Controller:

def show
  @article = Article.find(params[:id])
  @article_content = view_context.raw(@article.content)
end

View:

...
<li class="content"><%= @article_content %></li>
...

Essentially we're marking the Article content as html_safe (using the alias raw()) beforehand so it doesn't cause an issue in the view.

Messier than I'd like, but it works

Answer 3

If you are storing html on your model and you are on Rails 4.2++, you could consider using the sanitize helper (docs).

For example, you can allow specific tags (e.g. links):

<%= sanitize @article.content, tags: %w(a), attributes: %w(href) %>

The docs have a lot of good examples.

Here's another write-up if you want some more information.