search

Shade- / MyFacebook-Connect

A plugin to integrate Facebook with MyBB, letting users login and register through Facebook.
27 stars 24 forks source link

Fixed reflected XSS #8

Closed 1llusion closed 11 years ago

1llusion commented 11 years ago

When registration has failed, the variables weren't properly escaped and they could lead to reflected XSS.

Shade- commented 11 years ago

Not sure if adding this or not. These inputs have been intentionally left unescaped since the insert_user() method already escapes the username and the email when it comes to add the user to the database. Usernames can still contain the chars htmlspecialchars_uni strips out so it might produce unexpected behaviors when it comes to log in or similar.

1llusion commented 11 years ago

Please have a look at my edits. The inputs that are going to insert_user() are still left unescaped, the vulnerability exists in echoing of the variables when the username/e-mail contains invalid characters etc. (Notice that I'm only escaping when they will be echoed back, not before)

You can see, that the form breaks when putting "> as username and/or e-mail. (GitHub won't let me post any actual vectors :/)

XSS in this case can be dangerous as it could lead to account hijacking or worse. I've wrote a little article on how XSS can be dangerous: http://blog.1llusion.info/2012/12/killer-non-persistent-xss.html

Shade- commented 11 years ago

Unescaped inputs are actually escaped in the core with escape_string() method. Even MyBB's default registration doesn't an htmlspecialchars on the inputs because of the reasons explained above.

NewEraCracker commented 11 years ago

Isn't escape_string for database query escaping (aka escape input)? For output htmlspecialchars_uni is used as far I am aware of. Not sure if MyBB acts per design on this specific issue.

Shade- commented 11 years ago

My fault. Was thinking about SQL Injection instead of an XSS. I definitely need more sleeping hours every day. Merged.