Any danger in calling flash messages html_safe?

Question

I want a flash message that looks something like:

"That confirmation link is invalid or expired. Click here to have a new one generated."

Where "click here" is of course a link to another action in the app where a new confirmation link can be generated. Two drawbacks: One, since link_to isn't defined in the controller where the flash message is being set, I have to put the link html in myself. No big deal, but kind of messy.

Number two: In order for the link to actually display properly on the page I have to html_safe the flash display function in the view, so now it looks like (using Haml):

- flash.each do |name, message|
  = content_tag :div, message.html_safe

This gives me pause. Everything else I html_safe has been HTML I've written myself in helpers and whatnot, but the contents of the flash hash are stored in a cookie client-side, and could conceivably be changed. I've thought through it, and I don't see how this could result in an XSS attack, but XSS isn't something I have a great understanding of anyway.

So, two questions: 1. Is there any danger in always html_safe-ing all flash contents like this? 2. The fact that this solution is so messy (breaking MVC by using HTML in the controller, always html_safe-ing all flash contents) make me think I'm going about this wrong. Is there a more elegant, Rails-ish way to do this?

I'm using Rails 3.0.0.beta3.

Answer 1

It depends on how sure you are where the contents for the message come from. If there is any possibility that any user could manipulate that message, then you should not do this!

I wouldn't do it either way. Because it could happen that you now know that every string is safe, but than you change one controller and add a message which could contain user input, than you have a possible vulnerability.

I would set any message html_safe when it is added to the flash and you know for sure it is safe.

For example

class SomeController < ApplicationController def some_action flash[:info] = 'Some safe text!'.html_safe flash[:unsecure] = User.find(1).signature #//evil code end end

And in your view you can do it like this:

- flash.each do |name, message| = content_tag :div, message

This way you make sure that if you add a new flash message that isn't safe, it would be made safe in the view by mistake.

In this case the flash[:info] message is printed as html_safe and flash[:unsecure] will be escaped, so the user evil javascript code will not be executed.

If you know there is no possibility that there is any unfiltered user input in the message it should be safe to use html_safe on the flash messages.

Answer 2

I didn't want to tempt fate by html_safe-ing all flash messages universally, so I decided to just redirect failed confirmation link attempts directly to the url I would have linked them to anyway. It's a simpler, more elegant solution, I think.