buttonmen-dev / buttonmen

Buttonmen - an online dice game
Other
16 stars 24 forks source link

think about security problems which will affect the site once the firewall is down #627

Open cgolubi1 opened 10 years ago

cgolubi1 commented 10 years ago

Once we go to beta, we will not be firewalling access to HTTP (we'll probably stop firewalling alpha as well at that time, to avoid discouraging alpha-testers and spending time maintaining the ACLs). Once we drop the firewall, we'll be exposed to a lot of doorknob-rattlers and spiders (benign and malicious, but universally dumb). What bad things might happen at that point? Figure out what we should be most worried about which would be easiest to make small changes to prevent, and do so.

cgolubi1 commented 10 years ago

In case it wasn't obvious, i'd like some brainstorming here.

Some things i've thought of (other than the tickets i already filed):

Any other thoughts, or modifications to my thoughts? We can't do everything we think of here for the beta, but i'd like to hit the most egregious things, to reduce headaches at unexpected times later.

AdmiralJota commented 10 years ago
cgolubi1 commented 10 years ago

On logging: as part of #642, i excluded a bunch of known-harmless things from my script that greps the apache error log. Now it's back to showing real errors again, so that's probably good enough for now on that front.

cgolubi1 commented 10 years ago

Offhand comment: i keep coming back to the question of whether we're handing chat reasonably, but i actually think we are --- we're encoding chat for user viewing by setting the jQuery "text" attribute in the td in which we place the chat. The point of the text attribute is that it encodes everything as text, not as HTML. Under the hood, per http://api.jquery.com/text/, jQuery text() uses DOM createTextNode(), which seems to in fact be an approved safe way to excape stuff for rendering as text, see e.g. http://shebang.brandonmintern.com/foolproof-html-escaping-in-javascript/. I'm not saying this is foolproof, but in fact, i think this is The Right Way To Do it, and i don't actually need to worry about this issue again until we're doing forum-y stuff and trying to allow links within posts.

cgolubi1 commented 10 years ago

So, i did some reading/thinking, and at a high level, the things i think are dangerous right now fall into the general categories (some of these are bigger than others):

Which if any of those are worth doing anything about in the near-term? Still thinking.

blackshadowshade commented 10 years ago

I don't know exactly the details of this issue, but I would try to answer the question: "What's the worst that could happen?"

jl8e commented 10 years ago

Those are the major attack classes.

XSS: If the back-end is safe, then the front end shouldn’t need to re-check. If the back end’s been subverted, then everything it’s serving to the client is presumptively malicious, including the JS that’s supposed to check.

There should be a consistent policy of either sanitizing text for HTML entities either always on DB insertion, or always before sending it back to the user. I believe (I am in no way an expert here) that sanitizing before sending back is better:

While you’re probably right that your approach with the chat messages is safe, I’d be inclined to scrub them server-side anyway, just to have a consistent policy.

SQL injection: As long as all the SQL queries are parameterized, injection’s not a threat, no matter what weird characters are in the user-provided data.

If we’re using PDO, then there’s a catch, in that it apparantly fakes prepared statements on MySQL by default, though this is configurable. Here’s the fist link I dug up that contains how to fix that: http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php

CSRF: it's an issue, but there’s a relatively small amount of damage that can be done.

cgolubi1 commented 10 years ago

I created #674, #676, #677, and #678 for various security-related issues noted in this ticket. We definitely want to do an XSS/CSRF push, to be good browser citizens, but we don't think it's needed for the alpha.

Relisting this ticket itself as beta now.

AdmiralJota commented 10 years ago

Does the site still use unsalted hashes for passwords? If so, I strongly recommend salting them. If our database was ever exposed (e.g., if there were an exploit in the forum software we used), then it would be easy to recover passwords from an unsalted hash using a rainbow table. And just warning users not to re-use important passwords won't mean they won't do it anyway.

cgolubi1 commented 10 years ago

First off, i'm pretty sure we do salt our hashes. Second off, since multiple people have now mentioned breaking password hashes here, i'd like to caution against worrying too much about this --- i know it's very traditional advice to assume an attacker can get their hands on our encrypted password database and try to crack passwords, but, really: we send passwords unencrypted, we don't limit password-guessing attempts, we don't think we're very secure against browser-side XSS/CSRF-type games. People are going to get at user accounts using one of those methods.

Maybe they can also exploit our code bugs to execute arbitrary code on the server, grab our password database, and crack it offline. But the nice thing about AWS is that we're protected by their firewall, through which the only non-HTTP protocol we're allowing is SSH from a limited range of IPs using public keys only. So if they want onto the box, they're going to have to go through the webservice to get there. And, i mean, they might well. But i don't think it's the optimal way to specifically get at user passwords by grabbing our custom database format --- other things are so much easier and less dependent on our particular site/codebase.

IMO, worrying about cracking encrypted password tables is worrying about the wrong thing.

blackshadowshade commented 10 years ago

So, can we close this one?

cgolubi1 commented 10 years ago

I think the gist of https://github.com/buttonmen-dev/buttonmen/issues/627#issuecomment-37131634 is that we'd like to do a push of looking for XSS issues, and haven't done so yet, nor made a separate ticket for it.