We have a few things to talk about regarding SQL and PHP. It’s short and sweet, so I’ll be brief.

I’ll start with saying that I’d like to put this tactfully, but unfortunately, I don’t know how else to say this:

It’s 2014 and we’re still not using parameterized statements.


I ran across a project that was linked from one of the various “discover Github” mailings I’m subscribed to that looks very promising. It’s called Lychee, it looks great, and the demo functions well. I can’t think of any other open source gallery systems at the moment that look as good as Lynchee (or as simple). There’s just one problem: It’s an absolute haven for SQL injection attacks. Gosh, I don’t even know where to begin. Nearly every single query that accepts some form of external input is a potential landmine of shame, sadness, and roasted pandas. Worse, the last ticket addressing this particular issue was posted about a month ago. If there’s no activity this weekend, I might just go through and convert it to use parameterized statements myself and offer a pull request.

The other problem I have with it is the code formatting. It’s pretty ugly. It certainly wouldn’t pass PSR-2, and it reminds me vaguely of the ad hoc formatting found in most other popular PHP products (commercial and otherwise, so it’s not like Lychee is unique). But, that’s okay. Code formatting is mostly subjective, as long as you’re following some standard (even your own) and remain consistent. While some of us would rather split hairs over various stupid things, it’s mostly incidental because we care about appearance. Appearance does matter, because care and consideration for how the code looks probably reflects how the code operates. Pragmatically, though, appearance doesn’t matter that much. It can be fixed with automated tools. No biggy.

However, the SQL injections are unforgivable.

There are hundreds of resources on the matter, many targeting PHP. So it’s not like there’s much of an excuse. Heck, I’ve written about this in the past, and I’m not even a smart dude. If you haven’t heard of parameterized statements, either you’ve been living under a rock or you’re new at this.

The good news is that if you’re new to the big bad world of web software, don’t fret. The absolute best place to get started if you’re writing PHP is to read PHP the Right Way. It includes dozens of tips and tricks, information on established and emerging standards, and delves into a few advanced topics for larger applications. I would highly encourage you to read the entire thing. In particularly, I would suggest reading the section on databases, because it covers PDO, SQL injections, and parameterized statements (often known as prepared statements). There’s nothing wrong with the MySQLi extensions per se but new PHP code should use PDO instead. PDO even has a lesser known feature to inject queries into objects, which can take care of some of the more mundane work (if you happen to swing that way).

Don’t get me wrong. It’s great to see new projects popping up all the time challenging older, more established ones. It breathes new life into under-served markets and creates a greater body of useful tools for all of us. Sometimes a fresh look into old concepts helps us think outside the box, questioning established design principles, and jarring us back into the reality that there might actually be a better way of doing things. Please don’t feel this post is an outright criticism: It’s not. We all had to start somewhere. If you’ve never heard of SQL injections, well, now you have. Extra information in your arsenal of tools won’t hurt, but it certainly will help combat one of the most common design flaws in software today.

What you should take away from this post is that there are unsavory individuals out there who are deeply interested in easy-to-exploit flaws in software. These folks gain a great deal of enjoyment from imposing misery on others, either by writ of outright damage or embarrassment. Because of them, we have to develop for the web differently than we might have, say, 10 years ago.

So let’s start by teaching the newcomers not to repeat our mistakes!

No comments.

Musing about… Prepared Statements

A spurt of curiosity this evening–more specifically, one of those circumstances we each have from time to time wherein a handful of unrelated thoughts flutter about the conscious mind like a pair of butterflies flitting from flower to flower–consumed me sufficiently that I decided to do a brief Google search on prepared statements. I’m unsure where such a motive originated, but I’m fairly convinced that it was at least tangentially related to some misinformation I’ve heard of late related to web programming advice and also possibly due to my surprise that few commercial PHP bulletin board packages actually use prepared statements.

Before I begin, let’s consider for a moment that last and most disconcerting statement: Few commercial PHP forums use prepared statements. To the uninitiated, this might seem to be a matter of nick-picking unimportant to the real world. To the rest of you, it may come as a sad commentary on the state of modern programming and commercial software (perhaps, fittingly, as a commentary on the average run-of-the-mill PHP programmer). Prepared statements certainly aren’t new, and while they’ve been a part of PHP for a number of years now, it’s infuriating that they hardly see common use.

PHP first shipped PDO with PHP 5.1 (available as a PECL extension for PHP 5.0, circa 2004-2005). Intriguingly, for systems that don’t provide PDO support (or the appropriate drivers for PDO), the MySQLi and PostgreSQL functions and classes have provided prepared statements for quite some time, and the SQLite 3 drivers have provided a prepare() method since PHP 5.3. Commercial bulletin boards, like vBulletin and IPB, have seen many revisions in the years since, and several free/open source packages including phpBB have been part of similarly major overhauls. Yet the overwhelming majority of them still make no use of prepared statements. Humorously, as of this writing, vBulletin does provide a misleadingly-named sql_prepare method in its database class, but it doesn’t emulate prepared statements–it simply provides an escape wrapper with data type introspection and casting.

PDO has been available for nearly 8 years and many RDBMS drivers for PHP have offered prepared statements for at least that long (longer in the case of PostgreSQL if memory serves correctly). Yet every year or two, new major versions of popular PHP message boards are released, and every major release sees the same legacy database code under the hood. Perhaps it’s intentional. Perhaps the developers still want to support PHP 4.x in spite of the fact that it went EOL in 2008. Perhaps they just don’t know any better. Who knows!

Why Prepared Statements?

A prepared statement or parameterized statement, as it is occasionally known by in DBA parlance, is a specially-formatted SQL string that utilizes placeholders, either question marks (?), special named parameters (such as “:name”), or other database-specific strings, to indicate to the database or the driver where data is to be inserted. This has the benefit that, in theory at least, any data managed by a prepared statement is unlikely to serve as a vector for SQL injection attacks. The reason this works is because most drivers dispatch the prepared statement and its data separately on the wire and process them independently providing a certain degree of isolation. But wait, there’s more! Because of the implementation nature of prepared statements on most platforms, the query planner can often optimize and partially compile the statement such that, if it runs again, much of the legwork has already been completed and the query can run faster. Software like forums or blogs often execute the same query multiple times–with different data–so one might think it would be a natural fit. If it’s such a good thing, why do so many popular packages forgo such a benefit?

While I can’t answer for many developers, I think I know what at least part of the answer might be. First, for enormous code bases like vBulletin (and phpBB to a lesser extent), virtually no effort is made to separate the application logic from the underlying model. I’ll be fair in my distinction: The presentation layer is thankfully separated from the mess in the form of templates, but the remaining code is a bowl of spaghetti not unlike that of many of the very first PHP applications (and Perl!) that first graced the Internet over a decade ago. Because the model (and, by extension, the SQL) is so deeply entrenched in the functional logic of the application, reworking it to use prepared statements–and consider, also, that many of these queries are generated programmatically–would be a tremendous undertaking of many man-hours. Cleaning up the code properly such that it is more of a structurally sound framework (think MVC) is most certainly out of the picture. It isn’t impossible, of course, but when you consider that some functions in many of these software packages have persisted since the dawn of time, such refactoring becomes the thing of fairy tales.

To illustrate some of my displeasure, vBulletin version 4.2 still provides an iif function which is little more than a wrapper for the ternary operator (?:) in PHP. The ternary operator has been around since at least PHP4, yet there it is, in all its glory, a legacy function still available from the early days of PHP3 when such a beast didn’t exist!

One might think that it would simply be a matter of adding some logging code to old function calls, tracing the source that called them, and then reworking the culprit code to use built in language features. It might even take less than an afternoon.

The Drawbacks Programmer Mistakes

While prepared statements (parameterized queries for those of you who are embarrassingly excited by more elaborate verbiage) aren’t a panacea (I did it again) for all things SQL injection-like, they’re a good mitigation strategy, but it’s important to use them with caution. As Jason Lam states on the ISC Diary, “I still remember 4-5 years ago when SQL injection just started to become popular, the common mitigation suggested [was] to use prepared statement [sic] as if [they were] a magic bullet. As we [now] understand the SQL injection problem better, we realize that even prepared statement can be vulnerable to SQL injection as well.”

Well, yeah. This is where I smack my forehead. Maybe I’m being overly critical as I re-read a post from 5 years ago, because I’ve had the tremendously good fortune of witnessing some magnificently terrible code in my time as a web application developer.

Mr. Lam goes on to explain the insertion of unchecked user input, but I can’t shake the feeling that there is an implicit overtone in the article that it is somehow the fault of prepared statements. Perhaps more accurately, the article is faulting most of us for having championed prepared statements as a welcome solution to a very common and widespread problem. Realistically, though, it’s not an issue with prepared statements–they work just fine. It’s an issue with developers inappropriately using the tools at their disposal and doing so in a manner that simply transfers the vulnerability from query() to prepare() by forgetting to properly manage incoming data. Though, I should say that I’m inclined to suggest that programmatically assembling a prepared statement is somewhat counter-productive. More on this later.

Ironically, while doing some research for this article, I ran across a couple of posts on Stack Overflow that presented this problem of unchecked user input as one of the primary drawbacks of prepared statements. Really? Drawbacks? If you’re not using named parameters or placeholders for your query data, you’re probably not using prepared statements correctly! But drawbacks? Gee, maybe we were a little too vigilant in telling people to use prepared statements–so much so that they did a find/replace for query and swapped it with prepare. (I’m being facetious; so, to head off any comments to the contrary, it’s not at all possible to simply swap some text, because prepared statements do require a little more work.)

The problem I have with labeling unchecked user input as a drawback of prepared statements is that it is no longer a “real” prepared statement whenever such data is concatenated with the resulting query. Yes, it is still a prepared statement, insofar as calling prepare() on the driver’s end, but it’s no longer being used like a prepared statement. Here’s a hint to new developers, particularly PHP developers since a huge percentage of them are guilty of doing stupid things like this: Never concatenate unchecked input in any query–prepared or otherwise. If you’re using a prepared statement, use it like a damned prepared statement. The moment you start piping data into the query string itself, it’s no longer going to have the benefits of a prepared statement. (I’ll give you a special exception if you’re using LIMIT and concatenating integers with your queries since not all of you may be running MySQL 5.0.7 or later.)

Will the Real Prepared Statement Please Step Forward?

In my mind, and trust me, it’s a very strange place in here, a prepared statement is one that may contain parameters and is “prepared” ahead of time for reuse (that is, compiled) by the driver or the RDBMS (usually the RDBMS). Nothing more, nothing less. The instant some unfiltered data is slapped on to the end of the query, it’s no longer a pure prepared statement; instead, it becomes a mistake. Again: Prepared statements are parameterized queries that are usually compiled by the backend for a little extra speed. A query can contain anything else that the programmer adds into it, but fundamentally, a prepared statement is something that dictates a very specific structure. It certainly cannot overcome the mistakes of a naive developer who, believing that a prepared statement will magically fix all of their (singular they, sorry linguistic purists) security-related woes, use such a tool in addition to dangerous techniques like concatenating unchecked input. Another way to look at it is thus: If prepared statements are prepared (that is, compiled) by the database for reuse, and the developer is concatenating a dynamic value to the statement, the entire benefit of preparing (compiling) that statement is immediately lost, because the RDBMS has to re-compile the statement every single time it’s sent along the wire. Please, don’t do this.

Of course, there may be reasons not to use prepared statements all the time. For one, prepared statements in MySQL versions prior to 5.1 can no longer be managed by the query cache which may impact performance (High Performance MySQL, 2nd ed., p229). DBMSs that don’t support prepared statements can, in PDO at least, can have them emulated by the PDO driver at the cost of some pre-processing performance, and using older PHP functions like the popular-but-now-deprecated mysql_* ones just outright don’t support anything but basic queries (they also don’t use the binary interface, making them somewhat slower). If you’re using only a single query with absolutely no intention of reusing it, prepared statements may incur some overhead since the query must be compiled. Furthermore, for MySQL at least, if you’re not using stored procedures, the database has no way to share compiled prepared statements among multiple connections. Yet, while a prepared statement is no substitute for caution–particularly with programmatically-generated queries–it is a useful tool in the developer’s arsenal to protect against attacks like SQL injection. Plus, if you make it a habit to use PDO (you should), not only do you get emulated prepared statements for databases that don’t support them, you also get to use the modern MySQL APIs under the hood and some consistency, which says a lot in the PHP world.

Tangentially, this is also why it boggles my mind that many sites (banks, news agencies, airlines, and even some game companies) limit what characters the user can enter for their password, and how so many companies with an online presence often have draconian limits of less than 16 characters, inclusive. Seriously: If you’re correctly storing a secure hash of the password (HMAC, bcrypt, scrypt, or at least SHA256 or similar), you don’t need to store the password directly, nor does it matter if the password is 5 or 500 characters. It’s going to be comprised of a fixed length of some limited set of ASCII characters representing hexadecimal numbers which can be stored without much fuss. The 1990s were over two decades away. I think it’s time we stopped writing code like Y2K is still a viable problem.

Also, let’s start using prepared statements a little more often.

1 comment.