note_failed_signin unescaped string

[mjc-gh]

Line 40 of the note_failed_signin in generators / authenticated / templates / controller.rb

flash[:error] = "Couldn't log you in as '#{params[:login]}'"

Is unescaped and could lead to a potential XSS attack where an attacker could steal the users credentials. It should be:

flash[:error] = "Couldn't log you in as '#{CGI.escapeHTML(params[:login])}'"

[saizai]

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.

[mjc-gh]

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.

[saizai]

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. ;-)

[mjc-gh]

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.