Having been through open sourcing an app platform repo (https://github.com/documentcloud/documentcloud ) that wasn't explicitly created w/ open sourcing in mind, moot has my deepest sympathies.
Moot left out the most sensible response to Mistake #5:
I went through a rushed 8hr rebase to remove all of our keys, certificates, cookie secrets and the like before DocumentCloud's platform was opened. Two days after, a friend asked me why I hadn't simply revoked and replaced all of our secrets to the sound of me facepalming repeatedly.
It is always much easier just to revoke and replace your secrets. Doing so should always be relatively easy for you to execute, and having a chance to practice that is also always good.
Good point -- frankly I'd suggest doing both (clean repo and revoke/replace the secrets). The developer in question simply overlooked the issue, and since we were busy spinning down the company, nobody else caught it.
I would scrub the repo anyways because revoked secrets can still be used for social engineering. "Hey $HOST, I got hacked. My old password was $PASSWD and you can be sure it's me because $PUBLIC_INFORMATION. Can you send a password reset to $ATTACKER_EMAIL?"
Its a very good point that if you are not able to easily revert and change all your secrets and key's then you are in more trouble than just removing them from the commit log.
Of related interest to HNers: Admin consoles getting owned up is a fairly frequent way that even security-conscious shops lose their web apps / databases / networks, because one often does not code defensively against malicious actions executed by "trusted" users. You can mitigate this quite a bit by putting them on a private network interface and requiring the admins VPN in to use them.
Speaking as someone who got owned even when using SSL client certificates (made mistakes setting things up, long story), I think patio11's suggestion is better. Put it on a private network, seriously it isn't that hard.
The problem is not that this query was not escaped, the problem was that: 1) unparametrized queries are allowed in code, 2) user input is obviously not properly checked in all cases and 3) that WAF (Web Application Firewall) was not used / set up properly.
Other than that, thanks to OP for in-depth writeup - it helps to be reminded from time to time why security matters.
Not using placeholders should be an automatic fail of any code review. Period.
Without placeholders the risk is way, way too high. A single mistake can be the end of your entire application if not career.
Don't think "Oh, this is just test code, I'll fix it later" and leave a vulnerability in your application. Always write using placeholders. No exceptions.
Are you implying that there's a way to get past escaping...? That's a new one to me. I thought escaping was precisely so you can put any value in a string and have it still work.
Yes, but you had better be 100% sure that your escaping function is completely reliable, and the server hasn't introduced some new syntax since you wrote it that you aren't escaping properly.
I would trust parameters much more (although I have used proper escaping in the past).
A few popular database drivers use escaping under the hood for parameterized query arguments. mysql2 ruby gem (and any rails stack on top of it) for example.
The disclosure of these mistakes is both kind of relieving, in that the errors were obvious in nature, and terrifying, in that even though they seem "basic", they were still committed by a dev as experienced as moot. This would be a good post to create a checklist from, if nothing else.
I wonder how old the code was for "Mistake 2"? SQL injection is something most sites have patterns/frameworks to prevent, but unless the site started out with such practices...the code that was written when the site was just a fun side-project might go unchecked even as the site becomes a well-run project in its later years.
> they were still committed by a dev as experienced as moot
I'm not a terribly good programmer, and have been very hands-off with 4chan's code for quite some time. I still direct development and am responsible for the servers/sysadmin tasks, but there are far more talented developers out there than I. In the case of Canvas/DrawQuest, I was 100% uninvolved on the tech side.
But again, in both cases I accept full responsibility for the breaches since ultimately it's up to the project leader to ensure these things don't happen -- even if not active on the technical side.
> I wonder how old the code was for "Mistake 2"?
Very new. It was in a once-off file that we used to quickly pull stats about reported posts, which a) shouldn't have been on a domain without HTTP auth, b) should have been deleted long ago, c) shouldn't have had a bugged auth check or injection vuln to begin with.
one-off files have a habit of hanging around and temporary often becomes permanent.
This isn't to say that you should treat all one-off's and temporary solutions as permanenent but it is a good idea to audit them periodically.
Storing that kind of metadata about code is something I've often pondered we could do better, putting it in comments is a nasty hack, storing it away from the code means it instantly gets out of date, commit messages are not a good place to put that stuff either.
I've never come up with an elegant solution even in my head but it would be something I'd love to have for my own uses.
Someone once came into my office and asked why the email export feature had stopped working. Once they described going to test.php, I realized that about a month ago, I had migrated our version control system to a new deployment system, and hadn't included test.php, what I thought to be an insecure relic left hanging around by a predecessor.
Things that end up on a live web server are one offs much less than the people who make them think.
Codebase I once worked on, I found a /csv route that dropped the entire customer database in CSV format and /route_csv that enumerated all the routes the application had including admin and cron routes :| (denial of service by spamming the cron routes that did no access checking was the least of it).
When I checked the commit date it was 19 months ago..and in production for 17 months :|
The midden and the windmill fully hit each other that day.
How do you query your database?
There is still a bunch of new PHP projects that use mysqli::query, which is like playing with fire compared to using PDO::prepare
If you are still using mysqli, changing to PDO for future commits might help reduce chance of creating these kind of vulnerabilities.
Thanks for this Moot, I love the fact that people are being more and more open with how they were hacked. These post-mortems help others identify areas where they may be vulnerable which in turn helps the community as a whole.
Sorry about you getting hacked twice in one day though. I hope things have been better since then!
If I'm not mistaken, they took on $3.6 million in venture capital, and that might be the complication in deciding exactly what to do with the product (that is, their partners in it may disagree with selling it for $500k or whatever it might fetch).
Our investors have been incredibly supportive -- especially these past few [rough] months.
The biggest concern I have is not so much the price it would fetch (which is probably not much given recent events), but rather finding a suitable steward for the community.
I'm pretty sure there was someone who could step up. Product owners usually over value their involvement and thus never give up the reigns, going down with the ship instead of letting someone else take over.
I don't know. Community members usually undervalue the product owner's involvement as well.
Stewarding a community is a difficult job. You need more than just passion and I imagine finding one and feeling confident that they'll do a good job is quite difficult.
Security is a usability nightmare. All the instinctive and usually lazy things people do are just bombs waiting to explode. I don't know why this continues to be the case. Most platforms for web development should have taint checking turned on to the highest setting by default. SQL access points should scream in your face every time you use a bare query. With a bit more you could even get developers to do the right thing in terms of salts, passwords, and hashes. I think it is clear that pointing people to OWASP and asking them to read their security articles before bedtime is not as useful as it could be. Library writers also need to be involved and use safe options by default instead of opt-in.
The list of vulnerabilities in 4chan read like a standard web hack CTF. I'm surprised a site as big as 4chan has multiple OWASP top 10 issues, especially after the source was leaked. Storing credentials in client side cookies? Who wrote that shit?
4chan has always operated on a shoestring budget. When you have a single dev, there's nobody around to catch stupid mistakes. I just spent two days trying to track down a bug only to realize I'd been adding elements to an array in a braindead manner. Stupid happens.
Nobody mentioned the fact that _you don't put secrets in a versioning system_. .gitignore the configuration files and have example files available, or use configuration files that can import the missing parts (the secrets) from an ignored file.
Another way is to put the secrets in environment variables outside the project.
In any way, if they're missing provide a meaningful message to the developer.
I do understand that you must keep some of these secrets available somewhere: just don't version them.
Some security bad practices, but it happens to the best of us. Glad you got that figured out though.
I always tell my devs on the first day, "leaving AWS keys in a public repository is as bad as showing up drunk to work". Mainly because the last guy to leave our AWS keys in the open was drunk at work.
its really sad to see how many AWS Keys are public in github right now. A quick github search reveals many, many key pairs that were checking as recently as this week.
Amazon are apparently scanning themselves. If they are doing this then they might figure that the quantity of exposed keys will undermine their reputation. That is quite something in itself.
Amazon AWS support have (at least on here) a reputation for refunding fraudulent usage that stemmed from compromised keys. If that is in fact a policy they follow, it's in their best interests to cut down on leaked AWS credentials.
I'm guilty of this for an old project and only discovered when Amazon sent me email telling me it was public so Amazon seems to be searching Github (the web?) for mentions of the keys.
This makes me wonder what kind of practice change would be best to help avoid this sort of thing. Maybe anytime you put in something that you know is hacky and insecure as a one-off, you should put a note in wherever you would look regularly, and maybe some kind of reminder too, to make sure that your one-off is either removed or properly secured reasonably quickly.
Or maybe just a rant on PHP and/or frameworks that make it easy to do the wrong thing, and hard or time-consuming to do the right thing.
Looks like a good opportunity for an open source tool that scans a git repository for interesting/private information. Obviously it could be used for nefarious purposes, but it would be helpful for anyone looking to open source an existing repo.
Another alternative is to store credentials in environment variables :)
Probably a good practice. For my current project, I've been careful to never commit any API credentials or other secrets to the repo, even though I don't currently have any intention of making it public. It's harder to know what you're missing, though.
Did something similar, just built a module for our CI tool that checks against sensitive information (secret, salts, hashs, hidden feature), pretty efficient as every dev (myself included) can get lazy and use keys directly in scripts, instead of our Config loader that safely retrieves all that.
Not bad in itself, sometimes all you need is a dirty script, but as other have said, they tend to stick around, a key part of our CI module is actually putting expire date on scripts, or check dates. I will receive an email telling me to check and a warning on the CI at build time. It's been great to keep our code clean while allowing us to still do things dirty when required.
I actually think this is something GitHub should help with. When you flip a repo from private to public, it should prompt you asking if you want to wipe out all the version history first.(essentially create a brand new repo in the background)
Seems like far too common a mistake and something they could help fix once and for all.
So Mistake #4 is something you hear over and over again is bad but I don't understand why it's so bad. If you have the password you can just login anyway and I don't see how it affected this particular case either. Please enlighten me.
Normally, if I understand right, the point of hashed passwords, is what is sent from client to server to auth is the ACTUAL password.
Then the actual password is hashed, and the hash of the input password is compared to the previously stored hash, to see if it matches.
This way, you need the actual password to auth, but the actual password is stored nowhere by the app, only the hash is -- so the actual password can't be stolen by getting access to the db or the file system or anything else, because it's simply not there. But it's what you need to auth.
This is really the whole point of storing hashes rather than original passwords. Right?
Now, I guess that would mean you'd store the _actual_ password in the cookie. But that does sound risky, even if the cookies are https only. Which is why usually you store a _session id_ in the cookie, and auth the session with a single auth action, not store the password in a cookie.
If I could upvote you multiple times I would. I see a lot of confusion on this matter, even on HN.
When authenticating, the server needs to do a hashing step to compare against the password database. Otherwise, it isn't a hashed password database - it's a plaintext password database.
The cookie should not contain something computable at all.
It should be some kind of session identifier, generated by your code when user:pass combo checks out.
And if possible, store the session information somewhere else than your database. Redis and Memcached is a nice fit for stuff like that.
Well, the cookie should not contain anything computable by the user. You could use an encrypted cookie to store session information to save a round-trip to the DB. In that case it should be impossible for the user to derive the key or modify without detection (so HMAC-SHA256, GCM, Poly1305 or whatever authentication mode is fashinable/applicable.)
My proposed solution only ties the cookie to the IP, so the user will have to login again if their IP changes. But this means that even if an attacker who doesn't know the password gets a cookie, unless they have the same IP they wouldn't get access.
moot, why do you refuse to speak to us on 4chan while go happy hand-in-hand with all these web 3.5 cloud entrepreneurs here? Want to find some investor friends? Thanks for ignoring your actual friends.
Using someone's site does not make you their friend. That remains true even when you both enjoy some aspects of the culture. That said, his threads on 4chan are probably exhausting. It isn't like he's keeping anyone in the dark, either.
Okay, so I'm guessing this is an example of "feigned surprise" that is supposed to be considered bad?
I've been on a few sites (very recently) that just stick the password right in an auth cookie, served via HTTP no less. Luckily they're sweet enough to base64 encode it for extra security.
I would have expected a site like 4chan, the source of all sorts of script kiddie idiocy, would know better by this time than to stick a password, even hashed, in a cookie.
But sometimes you piss off the HN bees nest, and downvotes come streaming in. Reminds me of Reddit.
I think the original script it was based on didn't even have login per se. You just occasionally stuck in an admin password hard coded in the PHP to do a one time post delete or whatnot. It's mostly an anonymous forum after all. As more moderation functions grew a real login was needed and grafted on. Users still didn't have a login for a long time and instead had a special part of their username that was hashed when displayed to prove identity.
> But sometimes you piss off the HN bees nest, and downvotes come streaming in. Reminds me of Reddit.
No, just look at your comment. You quote the OP saying it was a boneheaded move to do X. Then you use feigned surprise ('cause really, who could be literally be so surprised that they type in half-words?) to express that it was a boneheaded move to do X.
Why bother to write that out, beyond "I thought this"? Why should anyone want to read it?
It's not you. HN creams their pants at the sight of moot.
If this was anyone else it wouldn't been something more along the lines of "this is why you should leave security to the experts" or "seriously, just spending an extra day reading about the best practices would've solved the whole thing"
I think people who are downvoting this comment are projecting what I meant by "so simple." There was no judgement on that statement. It was more an observation relating to that it could happen to anyone, and that they were "lucky" the person who got into their system wasn't really malicious.
Yes; upvoting just puts the better comments nearer to the top. Downvoting the fluff comments (with the minimal "effort" of a click while you read past) actually greys them out, which both discourages people from posting fluff in the future (with a minor karma hit), and makes it obvious to other readers that they've made it to the end of the real comments.
Moot left out the most sensible response to Mistake #5:
I went through a rushed 8hr rebase to remove all of our keys, certificates, cookie secrets and the like before DocumentCloud's platform was opened. Two days after, a friend asked me why I hadn't simply revoked and replaced all of our secrets to the sound of me facepalming repeatedly.
It is always much easier just to revoke and replace your secrets. Doing so should always be relatively easy for you to execute, and having a chance to practice that is also always good.