SQL Injection Security Issues

April 28, 2004 2:19 PM

SQL injection security under php/MySQL. What specific issues do I need to deal with? [more inside]

I've done plenty of Google searches, and read several print articles on the topic, but I still feel like I'm missing something.

If I validate all user input to make sure quotes are escaped and numeric variables really are numeric, have I closed all the sql injection holes? I think so. Since mysql_query() will only submit one query at a time I'm not too worried about people substituting "id=1;delete from users" for "id=1". Is that a valid assumption?

More to the point, should I sleep soundly at night if this is all I'm doing to thwart sql injection under php?

function MakeInjectionSafe($string,$mode,$MaxLength) {
if (($mode == 'int') and (!is_numeric($string))):
$string = 0;
else:
if ($MaxLength) $string = substr($string, 0, $MaxLength);
if (!get_magic_quotes_gpc()) $string = addslashes($string);
endif;

return $string;
}

Also, if I'm wrong about this, pretty please don't truncate my database.

posted by y6y6y6 to Technology (17 answers total)

It may also be worth your while to familiarize yourself with privileges. You can basically do a lot of tinkering with them to restrict user access to specific operations on specific columns for specific users from specific hosts. So if you're worried about people messing with your database, you can basically set up a MySQL user that only has insert access, and only has insert access to the table in question, and only from the host that your script is located on, and have your php script connect as that user. My only other tip is to make sure you remember to FLUSH PRIVILEGES every time you change them, or your changes will not take effect.
posted by alphanerd at 3:22 PM on April 28, 2004

Related: where do people find out about what security holes there *are* in PHP, and how to close them? The only one I've ever heard about is this injection attack, and making sure you use $_POST instead of register globals.
posted by gramcracker at 3:24 PM on April 28, 2004

There was a decent thread on /. yesterday on this topic. A couple of highlights:
#1
#2
posted by yerfatma at 3:33 PM on April 28, 2004

MySQL recently implemented support for stored procedures. It's a much better architecture than sending sql strings through commands in web scripts - for lots of reasons including offering better protection against sql injections.
posted by normy at 3:35 PM on April 28, 2004

I'm not much of a PHP expert, but I think you'd probably be better off using mysql_escape_string, which is closer to perl's $dbh->quote(), and escapes more than addslashes.

gramcracker: Try bugtraq.
posted by fvw at 3:40 PM on April 28, 2004

alphanerd's advice can be a pain to implement at first, but it's a very good practice. If you're limited in the number of user accounts you can create, at least give them strong passwords and the bare minimum of privileges they need to do their jobs.

People find security holes by looking for them and by stumbling upon them. I wouldn't think of SQL injection as one attack; it's more like a whole realm you need to be cautious of. Using $_POST instead of making register_globals do the heavy lifting is good, but I always take it a step further and immediately assign the posted value to a variable. That way it's locked away in a cage and can only do what I let it do (so if I'm sloppy and "let" it do something nasty by not preventing that behavior, that's my fault).

On preview: stored procs aid security, make things faster and make applications easier to maintain, so now I just have to wait until providers start installing MySQL versions that support them. Anyone have a good reference for MySQL stored proc syntax (other than the manual-- just looking for a favorite bookmark or something)?
posted by yerfatma at 3:40 PM on April 28, 2004

yerfatma, not very detailed, but a start here.
posted by normy at 3:49 PM on April 28, 2004

Response by poster: alphanerd - Agreed, running your scripts so that you give programmatic access to the DB super user is bad. But probably not helpful in my context. I'm writing something that I want to distribute. Specifically for web logs. A solution that requires bloggers to create special MySQL users may be a non-starter. Sad. But true. If I can lock it down without it that would be much preferred.

gramcracker - yerfatma's link #2 is pretty much it. However, it's not php that has security holes, it's the code you write that has security holes. So having a proactive attitude is better than a hard and fast list.

normy - Yep. Stored procedures are better. But I'm not using them. Part laziness, part preference. Again, if I can avoid going that route it would make me happier. Part of my problem is that I don't know what I'd fix in a stored procedure that I'm not fixing in the function above.

fvw - You are correct sir. Thanks.

Part of my problem is that this will be used by people to legitimately add huge blocks of text to the database. And these text blocks may legitimately contain valid SQL.
posted by y6y6y6 at 4:17 PM on April 28, 2004

Here's what I use, if you're interested.

/* Takes a format string and an list of arguments and substitutes the format
codes in the format string for the elements of the arguments list. Similar to
vsprintf, but specialized for generating SQL queries.

Format codes are denoted by a percent sign (%) followed by another character. To
insert a literal percent character into the output string, escape it (%%). Each
successive format code is replaced by its corresponding argument in the list.

%%: Replaced by a literal percent sign.
%s: The argument, which must be a string, is used verbatim.

%S: Replaced by the argument, which is escaped with addslashes.
%q: Use for quoted strings. Like %S, but the value is also surrounded with quote
marks.
%l: Use for LIKE strings. Like %q, but the _ and % characters are also escaped.
%d: The argument, which should be numeric, is used verbatim. Non-numeric
arguments will be converted into numbers first.
%n: Like %q, but empty strings are replaced with NULL instead of ''.
%t: Like %n, but converts the result to a timestamp with FROM_UNIXTIME().

Parameters:
1. The format string.
2+. Any arguments needed by the format string.
*/
function format($format)
{
$argument = 0;

for ($i = strpos($format, '%'); $i !== false; $i = strpos($format, '%', $i + strlen($replace)))
{
$value = func_get_arg(++$argument);

switch (substr($format, $i + 1, 1))
{
case '%': $replace = '%'; --$argument; break;
case 's': $replace = $value; break;

case 'S': $replace = addslashes($value); break;
case 'q': $replace = format('"%S"', $value); break;
case 'l': $replace = preg_replace('/([%_\\\'"])/', '\\\\1', $value); break;
case 'd': $replace = (int)$value; break;
case 'n': $replace = (strlen($value) == 0 ? 'NULL' : format('%q', $value)); break;
case 't': $replace = format('FROM_UNIXTIME(%n)', $value); break;

default: $replace = '';
}

$format = substr_replace($format, $replace, $i, 2);
}

return $format;
}

I use it like this: format('SELECT * FROM users WHERE id = %d AND userName = %q AND address LIKE "%%%l%%"', $id, $userName, $city)
posted by Khalad at 5:49 PM on April 28, 2004

Aach, I so carefully made sure to indent it properly and blew it on the final click of "Post." Here's the code properly indented. Sorry for taking up so much space on the page!

/* Takes a format string and an list of arguments and substitutes the format
   codes in the format string for the elements of the arguments list. Similar to
   vsprintf, but specialized for generating SQL queries.
   
   Format codes are denoted by a percent sign (%) followed by another character. To
   insert a literal percent character into the output string, escape it (%%). Each
   successive format code is replaced by its corresponding argument in the list.
   
       %%: Replaced by a literal percent sign.
       %s: The argument, which must be a string, is used verbatim.

       %S: Replaced by the argument, which is escaped with addslashes.
       %q: Use for quoted strings. Like %S, but the value is also surrounded with quote
           marks.
       %l: Use for LIKE strings. Like %q, but the _ and % characters are also escaped.
       %d: The argument, which should be numeric, is used verbatim. Non-numeric
           arguments will be converted into numbers first.
       %n: Like %q, but empty strings are replaced with NULL instead of ''.
       %t: Like %n, but converts the result to a timestamp with FROM_UNIXTIME().
       
   Parameters:
     1.  The format string.
     2+. Any arguments needed by the format string.
*/
function format($format)
{
    $argument = 0;

    for ($i = strpos($format, '%'); $i !== false; $i = strpos($format, '%', $i + strlen($replace)))
    {
        $value = func_get_arg(++$argument);
        
        switch (substr($format, $i + 1, 1))
        {
          case '%': $replace = '%'; --$argument;                                        break;
          case 's': $replace = $value;                                                  break;
                                                                            
          case 'S': $replace = addslashes($value);                                      break;
          case 'q': $replace = format('"%S"', $value);                                  break;
          case 'l': $replace = preg_replace('/([%_\\\'"])/', '\\\\1', $value);          break;
          case 'd': $replace = (int)$value;                                             break;
          case 'n': $replace = (strlen($value) == 0 ? 'NULL' : format('%q', $value));   break;
          case 't': $replace = format('FROM_UNIXTIME(%n)', $value);                     break;
          
          default:  $replace = '';
        }

        $format = substr_replace($format, $replace, $i, 2);
    }

    return $format;
}
posted by Khalad at 5:50 PM on April 28, 2004

Response by poster: Very cool. I hadn't thought of that approach. Thanks.
posted by y6y6y6 at 5:59 PM on April 28, 2004

SafeSQL seems to do something similar to what Khalad proposes.

Alternately, the new mysqli functions (though these probably aren't an option for you) support using bind variables which eliminate a lot of SQL injection possibilities.

Or a DB abstraction library (such as PEAR::DB) will usually offer quoting, binding and/or statement auto-generation mechanisms.
posted by sad_otter at 7:34 PM on April 28, 2004

use bind variables, and pear::DB. i can't stress that enough. if you start using PHP with other databases, the SQL injection problem is big.

also, the next version of MySQL is suppose to allow nested SQL queries which means that if you don't use bounded variables, and (assuming that this MySQL database isn't owned by you), you could get into problems. Once MySQL 5 becomes finalized and webhosts start upgrading, I bet there will be a lot of people complaining about lost database data, etc.
posted by Stynxno at 10:10 PM on April 28, 2004

if no user input is involved, can i just do my queries without any form of escaping? or is there some way even that would be unsafe (given that PHP is more or less == a web environment)?

have the early performance issues with pear::DB been resolved to everyone's satisfaction?
posted by danOstuporStar at 9:09 AM on April 29, 2004

without any form of escaping

other than addslashes() of course.
posted by danOstuporStar at 9:12 AM on April 29, 2004

What would you be escaping if there's no user input? At that point, I'd be worried the escaping would obscure a typo-type (ick) bug.
posted by yerfatma at 10:16 AM on April 29, 2004

Response by poster: This is all good stuff. Thanks much. Due to my own large ego I won't be using Pear or building my own db abstraction layer. Well, maybe later, but not right now. And some day in the distant future everyone will have php 5, but for now I need to assume 4.

Here's my plan. I think this will work:

1) Escape all quotes with mysql_escape_string
2) Validate all integers
3) Strip punctuation from strings that don't need it.
4) Strip white space from strings that don't need it.
5) (here's the biggy) In all text blobs, replace every dangerous SQL command with a place holder. Then when displaying text, convert the place holders back. So what gets sent to the database literally can't contain sql injection.

Sort of. They could still do sql injection. But they wouldn't be able to *do* anything.

danOstuporStar - Yes, but there are many caveats. If you have register globals on a hacker can inject values into variables. And if you're creating something from the database you need to worry about how it got there.
posted by y6y6y6 at 10:32 AM on April 29, 2004