Reddit.com's response headers:

    Cache-Control: no-cache
    Connection: keep-alive
    Content-Encoding: gzip
    Content-Length: 18170
    Content-Type: text/html; charset=UTF-8
    Date: Wed, 04 Apr 2012 04:50:39 GMT
    Pragma: no-cache
    Server: '; DROP TABLE servertypes; --
    Vary: Accept-Encoding

I put that in as a joke years ago. I can't believe it's still there.

I'm sure somebody somewhere is actually using that to clean up a table in some sort of wonky proxy setup and will cry foul if that header is ever removed.

I've tried to write a program (https://gist.github.com/392445) which would be affected by this. It seems it should be `'); DROP TABLE servertypes; --` to actually drop any tables. All I was able to get now is syntax error.

I had the same idea: https://gist.github.com/913402

That's not a valid syntax for INSERT in MySQL. When you use the VALUE or VALUES keyword, it must be followed by a set of parentheses.

This is a valid syntax for MySQL:

    INSERT INTO servertypes SET server = 'Apache'

I recently learned about the INSERT SET syntax, and have started to use it across the board. Sooooo much more readable when column names and values are next to each other rather than separated. </offtopic>

I once found this in live code which was -- irony alert -- checking Authorization headers. Something of the flavor:

"select user.* from users where TO_BASE64(email + ':'+ password) = '" + headers["Authorization"] + "' limit 1";

(Can't quite remember -- that base 64 bit might have been pre-calculated in a column. It has been a few years.)

One would hope that in addition to fixing the SQL injection they fixed the use of HTTP basic auth over a non-encrypted connection but, well, some stories are probably better left untold.

Not to mention storing passwords with a trivially reversible transform...

I once worked in a company where another team were responsible for the company flagship services which - after you'd logged in - stored your customer id in a cookie.

That was the only authentication for subsequent requests. And the ids were sequential.

After I demonstrated to them that I could get the CEOs personal details, as well as trivially brute force all other personal details in the system with a bash script calling curl, not to mention run up massive bills (the system allowed setting up phone conferences with up to 30 participants and let you use the web interface to call out to participants worldwide), they thanked me and told me they'd fix it.

Next day they'd released an updated version which they said encrypted the customer id.

Two problems:

No nonce or anything, so if you sniffed it once, you could still trivially use it to "log in" and continue as before.

Secondly, their "encryption" turned out to be base64.

I sent them a new script that showed them how to get the customer id's still. They were amazed that I'd "cracked their encryption".

This article does not provide a good pattern for protecting against SQL injections. You can protect against all possible input vectors by using placeholders. Constructing SQL queries with string formatting or concatenation should be avoided. When executing bulk queries it is also more efficient.

  $stmt = $dbh->prepare("SELECT user,password FROM admins WHERE user=? AND password=? AND ip_adr=?");
  $stmt->execute($_POST['user'], md5($_POST['password']), ip_adr())

The above approach does not require that you think about what data you are accepting, and if done through-out your code, others who may not be so pre-disposed to think about escaping will not make mistakes if they copy you.

The x-forwarded-for example is the perfect example to explain why you should just escape anything and never try to guess trust boundaries.

Sure. When you started with that query, that IP address likely was just REMOTE_ADDR (guaranteed to not contain "bad" characters), but then the app was put behind a reverse proxy and support for x-forwarded-for was added, suddenly changing the IP address to something user-modifyable.

It costs practically nothing to use your ORM or prepared statements in order to escape all values you put into a query. So just do it and don't try to guess whether you can trust a value or not, because, after all, the source of these values can change, sometimes even without you knowing.

It's also a great example of assumptions leading to bugs.

It's not a standard header, it's just a header... a string.

IP addresses may easily be IPv6.

You can easily have comma delimited lists of them.

You don't know how long the input will be.

And unless you are sure that your network is cleaning the incoming requests and setting those headers, you shouldn't even begin to think that they are to be trusted for anything.

This seems like a total non-issue. Just CORRECTLY sanitize everything going into the database!

<snark> By the way, just to let everyone know also. You should also sanitize form submissions. </snark>

Thinking about "sanitizing" puts you in the wrong frame of mind in my opinion - you should be thinking about ESCAPING. If you're constructing SQL queries sensibly (using an ORM or a library that replaces placeholders rather than concatenating strings together yourself) you won't even have to think about that.

In defense of the author, his audience appears to be mostly people doing black-box security scans (and those writing security-scanning software). For those people, understanding additional attack vectors is useful.

Or, more succinctly, don't trust anything that comes from outside your application stack.

Or use prepared statements regardless of the source so that knowing the source, and tracking changes of the source of variables, becomes a non-issue.

Heh, that's not how most people think. It's more like:

Security! It's a total non-issue! Why would anyone want to break my app?

Most people seem to feel this way until their apps are dumped, rooted, hacked, or they just end up thinking security is cool and say "Man, I didn't realize how much of a mess I had before."

Basic scans need to be part of the CI workflow of startups these days. The same QA tier you use for Selenium and what not you should just throw Nessus/SQLMap at and have injections/vulnerabilities of the web stack fail builds as well.

Completely agreed. And actually, this is a large part of what Tinfoil is currently working on building. If you have suggestions, we're all ears.

It's all too common to hear people not caring until its too late. At least with all the skiddies running around nowadays it's harder for anybody rational to ignore.

Ha. It turns out the majority of engineers who are not security-conscious don't do this, because it's easy to forget. And there's always another way to get around it, unfortunately.

Sanitizing wont take care of problems from concatenating sql queries, for example, rather than using parameterized statements.

If using PHP, use PDO and always use parameters.

Use HTMLPurifier to avoid xss.

You still need to be careful if building queries dynamically, such as dynamic WHERE or ORDER by clauses.

I'd take that one step further and use an ORM like Doctrine, because otherwise doing trivial stuff is completely annoying. If you don't use an ORM package you quickly start writing your own ORM anyway.

Agreed, but even some ORMs (like Yii's ActiveRecord) don't validate the field name in the WHERE part in certain functions.

HTMLPurifier? That thing should only be used if you are accepting html as input (eg: wysiwyg). Just escape everything in the view layer (not in the DBAL. There you only escape to avoid sql injection, not XSS!)

what if someone puts html in a plain old text field?

your db should never contain xss material. some jackass will forget to escape it on display.

The thing is, you can't just 'escape XSS'. You need to know exactly where the data is going to be used, and you can't know that when you store it (in fact, you could use the same data in many different places). There are infinite ways of doing XSS, so if you don't escape properly, you are still vulnerable. The view layer should escape everything by default. Then if you need a different filter, or disable escaping (for wysiwyg for instance), do it manually.

but how often is someone's username <script>stealMahCookeez()</script> ?

As well as SQL injection plenty of sites do odd things which mean they don't work for plenty of users. There are numerous sites which don't load when sent the header:

  x-forwarded-for: unknown

Whatever they were trying to achieve with this (I'm guessing some sort of reverse IP/geolocation lookup) the effect was that none of the people behind our corporate filter could view their site at all.

While building the web performance analytics system at Yahoo!, I took on a daily task to look through access log lines that our parsers didn't like. Things like strange user agent strings, strange referrers, etc. We'd find all sorts of injection attempts in the User-Agent. SQLi, XSS, Shell injection, windows executable injection. Attackers tried whatever they could.

If you ever send /any/ client input (or any variable) un-escaped to your DB, I really hope you get your tables deleted as reminder (of course you have a backup, so it will only be a reminder not a disaster).

After 15 years of web development, there is no reason why people still would make this mistake.

I guess this is one benefit of a NoSQL database...

NoSQL databases are susecptible to injection attacks.

Maybe they ment to say 'NoDatabase' database instead? ;)

Incredible