I just set up restful_auth with the cucumber tests on rails 2.3.5 and Ruby 1.9 and found that the user step
Then /^she should +see an? (\w+) message '([\w !\']+)'$/ do |notice, message|
response.should have_flash(notice, %r{#{message}})
end
should be
Then /^she should +see an? (\w+) message '([\w !\']+)'$/ do |notice, message|
response.should have_flash(notice, :content => %r{#{message}})
end
for things to work. Don't know if something changed recently that would have this work before but not now, but it all goes back to the have_flash which depends on have_tag, which in turn (so far as I can tell) looks for content if you send in content through a hash. Perhaps it used to look for any second argument as though it were content, but if so it is doing so wonkily now.
Yes-but:
a) you should be escaping your flashes at the view level anyway (rather than trusting to do it on every insertion)
b) h() is easier :-p
c) is there a situation where someone could insert to another user's flash, given that this gets added to the current session? They'd just attack themselves.
Yeah true, using h() would be easier. I often have my application template display flash error and warn and escaping them wouldn't be an issue. Some users may not want to escape every flash notice\warn however.
The way I envisioned an attack using this vulnerability was that an attacker would create a special link on some rouge page. This link would POST to the login page of the vulnerable application. The login parameter would be some JavaScript, which would in turn, alter the forms actions to point to the attacker's page. The attacker now can collect credentials and further more devise an attack that does not disrupt service or alert the user in anyway.
Hm, interesting attack scenario. Seems unlikely IMO, but also, everything should be prevented.
I think this is more an argument for (a) - you should treat the flash as tainted in your view, and escape or whitelist it appropriately.
However, it certainly doesn't hurt to escape it as you suggest. ;-)
I just come from the school of thought that it should be default, out of the box secure, with no extra user intervention or anything.
It is an easier enough fix as well, one extra method call the problem is no longer present.