The state of PHP security [LWN.net]

The state of PHP security

Posted Dec 22, 2006 17:00 UTC (Fri) by iabervon (subscriber, #722)
In reply to: The state of PHP security by denials
Parent article: The state of PHP security

Every programming language (with SQL bindings) does seem to use strings for SQL statements, and it's a design error in every one of them.

SQL string constants can have single quotes in them escaped as a sequence of two single quotes, but SQL statements can only have single quotes using weird other methods (for purposes like having a column named "don't", of course; none of the standard functions or keywords contain single quotes). And SQL statements can (and must) include single quotes around string constants, but I'm prohibiting writing string constants as statement text.

E.g., you'd have to write something like:

new Statement().cmd("SELECT uid FROM passwd WHERE password=").
                lit(password).cmd(" AND username=").lit(username)

And this would ensure that the two string literals are properly quoted and escaped. And the user can't do something like cmd("WHERE password='" + password + "'") because the single quotes in the command text are prohibited. And obviously cmd("WHERE password=").cmd(password) would never work, because the client-supplied password wouldn't get even the outer single quotes. So the correct way is the only way to get it to work at all, and you don't have any SQL insertion holes possible unless the language bindings get screwed up.

Of course, it's not necessary for the database to support placeholders in order for the language bindings to support substituted constants. The language bindings can substitute properly-handled constants, which would imply that any security flaws must be in the language bindings, which are exclusively written by people who ought to know better than to mess it up.

(Log in to post comments)

The state of PHP security

Posted Dec 25, 2006 4:39 UTC (Mon) by erich (subscriber, #7127) [Link]

Sorry, but that's just a hack to make users switch to a secure syntax.
And it especially prevents programmers who know about the security implications to make their code readable... e.g. by constructing queries in strings.

I used to have hardcoded statements such as 'WHERE email NOT LIKE "%@%"' and I'd sure prefer to keep them this way. Also note that with LIKE, you might need a different escaping (which eventually needs to escape %, too).
Having to use 'WHERE email NOT LIKE ?' and passing "%@%" as first parameter is fine with me, but don't force me to use that ugly pseduo-OOP syntax you suggested, with two different appends for the string buffer. Ugly!

P.S. sometimes you need quotes to be able to access certain tables or colum names. E.g. have you had a column named "like"? or "where"?

The state of PHP security

Posted Dec 25, 2006 6:08 UTC (Mon) by iabervon (subscriber, #722) [Link]

The company I was working for eventually ripped out all of the statements constructed in strings because they were too unreadable. It's fine if the query doesn't vary at all, but once you have any variability at all, either structural or with constants, it's more readable to have a smart buffer. Of course, the syntax should fit the language you're writing in (mine was Java, hence the StringBuffer method chain); maybe you'd rather

buffer = SQL("SELECT uid FROM passwd WHERE username=") + username +
  SQL(" AND password=") + password;

Incidentally, you're using entirely the wrong quotes. String constants have to be in single quotes (unless you're using old MySQL syntax), and column names can only be in double quotes (or, if you're using old MySQL, back tics). If you're using the same quotes for both string constants and column names that match keywords, you've got bigger problems than the library interfering (is "password" a constant or a column name?)