use a security token to prevent XSS

[Frug]

Implement a security token that is created when users log in and lasts for the duration of their session. All AJAX calls should then be required to include this token or they should be rejected as XSS attempts.

[caffeinewriter]

Just a minor note, this is actually to prevent CSRF, not XSS :stuck_out_tongue:

[Frug]

Heh, quite right. Whoever first reported it called it that and it stuck.

[caffeinewriter]

Regardless, it's a brilliant idea that should be implemented. :+1:

[jsebean]

You could also merely blacklist the usage of that URL (the logout URL) in the img code. The code changes would all be javascript only and would not involve in tokens or anything.

I can hack around with that tomorrow.

[caffeinewriter]

@jsebean While that might work to help prevent basic attacks, non-comprehensive blacklists are often bypassed with relative ease. CSRF is a more complete solution, but by all means, please take a swing at it :) Any security improvements are awesome!

[jsebean]

@caffeinewriter You are right. A simple blacklist idea is an ugly hack, but I believe the BBCode parser is all in JS basically, no? This would stop the specific logout bug that allows people to paste the logout URL in an img tag in chat without needing any core changes specifically.

A token is still ideal for sure though, it's just more work haha.

[jsebean]

Ok so I made a super stupid quick change that solves the logout problem. Literally just one line of code, not sure if you guys want it, but I made a pull request if you do.

It doesn't implement a proper security token system so it doesn't solve this issue per se, but it doesn't make any DB changes which is a plus on that regard. It fixes the bug where people can include the logout other chatters by including the logout URL in an img code.

I thought there was another bug report on here about that but I don't see it.

[Frug]

@jsebean caffeinewriter's comment is not just that it's hacky, but that it's not really secure because it's easy to bypass. In general, trying to stop these attacks with blacklisting fails because people find ways to trick your string search. For example, if I inserted a non-printing character somewhere inside logout=true it might fool your string replace, while your browser might ignore it and hit the url (logging you out). I had provided this solution in the google groups, if you search there it's buried in one of my announcements a while back. The text replace custom js can be used to cut logout=true from appearing anywhere in chat. It's just not a real solution. There's nothing wrong with using it. It will stop 90% of script kiddies from crashing your chat.

[jsebean]

True that, using a redirect service also would very well also circumvent it. ;) Like I said, writing a real token system is the best solution. Heck this would only be a very weak temporary cover up haha

On Fri, Mar 27, 2015 at 3:31 PM, Philip Nicolcev notifications@github.com wrote:

@jsebean https://github.com/jsebean caffeinewriter's comment is not just that it's hacky, but that it's not really secure because it's easy to bypass. In general, trying to stop these attacks with blacklisting fails because people find ways to trick your string search. For example, if I inserted a non-printing character somewhere inside logout=true it might fool your string replace, while your browser might ignore it and hit the url (logging you out). I had provided this solution in the google groups, if you search there it's buried in one of my announcements a while back. The text replace custom js can be used to cut logout=true from appearing anywhere in chat. It's just not a real solution. There's nothing wrong with using it. It will stop 90% of script kiddies from crashing your chat.

Reply to this email directly or view it on GitHub https://github.com/Frug/AJAX-Chat/issues/173#issuecomment-87044868.

God Bless, Jonah Sabean http://www.jsebean.com/

[jsebean]

@Frug just one question. With the logout thing, what's wrong with simply changing from GET to POST? I did that a while back on my own chat and it seemingly fixed it that one issue, but I thought I read somewhere it broke something? I wasn't aware of it at the time what it broke since every seemed to work fine for me.

The logout issue is just something that I think should be solved and released since it's a quite major annoyance. I've been in a number of chats where people have done it.

[Frug]

It breaks hanging around in the logged out page. The logged out page expects to see logout=true in the URL, otherwise it just logs you right back in. The security token is required even for POST requests, to be sure. I agree it could be a major annoyance. Personally I would just ban people, and add the basic filters looking for logout=true for now.

[adriweb]

I've found another vulnerability close to this one, however due to being not known AFAIK, I've contacted Frug directly, hoping for a quick patch :) Implementing CSRF tokens is a must!

@jsebean : Just switching to POST doesn't protect users from an HTML form that has the action attribute set to the chat URL, and the logout parameter set to true in the data sent. It could also work from JavaScript directly on older browsers that don't enforce the Same-Origin-Policy.

[Frug]

PR #235 resolves this for logouts specifically, although the potential remains for other commands.

[chuchichaeschtli]

Recently somebody froze my browser so I tried this bug to see if there us any relation.

A bit of a warning would have been nice ...

Whenever I enter now, the Chatbot is still in an endless login loop.

Is there something on the server, that needs to be killed? Some process need to be restarted?

[Frug]

clean out your database!