Hacker News new | ask | show | jobs
by ezekg 3465 days ago
This website is just asking for an SQL injection. My goodness.

Edit: I spoke too soon. The database has been dropped: http://gitpay.org/user.php.

2 comments

Well the PR got merged, and finally he responded to my original request via email:

"Im sorry I did not get back sooner as I and another family member have been unwell in bed most of the last week.

I'd like to you for spoiling my Christmas

Now, someone hacked the server and deleted the whole gitpay database.

This was just volunteer work, so that open source developers like myself who get no pay might be able to a tiny amount of donations.

I have now lost a huge amount of work.

I hope you feel quite satisfied."

That sucks that he didn't have any backups, but it was just a matter of time before it happened. But nobody hacked the server; you can literally just throw SQL into the url: http://gitpay.org/user.php?user=%27%3B%20DROP%20DATABASE%20d.... That's why you should never trust any user input.
Amazing, they've used prepared statements wrong every time: https://github.com/gitpay/website/blob/master/functions.php

Sadly, SQL injection vulnerabilities aren't a rarity for PHP sites :( It's too easy to make this exact mistake.

It amazes me that PHP (or the database library in question) allows you to call `prepare` on a non-literal string without even a warning.
So, let's drop PHP for a moment. If you were writing a database library in say Java, how would you know or prevent the user passing you a concatenated string over a string literal? Is it Java's fault that you can't (excepting major bytecode hackery maybe?)?
Education can go a long way. If you check the PHP PDO docs [0], the fact that prepared statements make SQL injection possible is only the second most important fact for them. The documentation for PDO::prepare does not mention this fact at all. It just says you can use placeholders. Great.

Or just use an ORM. They have a bad reputation, but SQLAlchemy + Python is an awesome combo. But because of language features, PHP ORMs aren't quite as seamless.

[0] https://secure.php.net/manual/en/pdo.prepared-statements.php

[1] https://secure.php.net/manual/en/pdo.prepare.php

> If you were writing a database library in say Java, how would you know or prevent the user passing you a concatenated string over a string literal?

Extend the language to detect passing a string literal to certain functions or macros. Rust does this for macros that take a format string, like "println!" and "format!". GCC can do this for printf as well. And Perl has taint checking.

It's too easy to misuse PHP that way. Though I think enabling E_STRICT would produce a warning.