Best way to prevent sql injection?

I have to disargee. Injection problem has absolutely nothing to do with validation. That is, ‘valid’ data can cause an injection, and vice versa.

Classical example:

DELETE FROM company WHERE name = ‘<input>’

<input> equal to XYZ’ OR '1 is obviously an “injection”, but it should pass all validation test, because it seems to be a valid Company Name (we need some kind of semantic analysis to prove the contrary).

mmmh… good point, kinda cutting-edge but interesting.

Ahh… I see. So as long as I have magic_quotes off and then addslashes() to my variables, I shouldn’t have to use strip_slashes() ever again. :slight_smile:

So, other than the other things related to sql injection discussed in this thread, I should be ok with this setup?

Thanks again.

Rob.

I use this function on all vars being written to a database…

function smart_quote($var)
	{
   		// Stripslashes
	   if (get_magic_quotes_gpc()) {
    	   $var = stripslashes($var);
	   }
   		// Quote if not integer
   		if (!is_numeric($var)) {
       	$var = "'" . mysql_real_escape_string($var) . "'";
   		}
   		return $var;
}

Roger Ramjet: Thanks, I must’ve overlooked your hint before.

stereofrog: For string variables, type-checking isn’t gonna do any good, since any incoming variable is obviously a valid string. That’s why these must be escaped, and you can’t inject anything through a variable that has been (checked and escaped) or (escaped and quoted). The escaping is always essential, other checks vary from person to person, by their prefered methods, but just escaping or just quoting or just checking is simply not enough.

CapitalWebHost: what about variables that were NOT a part of the request and have backslashes in them? If magic_quotes_gpc is on, these will also be stripped. Unless you keep track of which variable came in the script via request and only escape these when building query. I deal with this with the method that duuudie wrote a few posts above - all incoming variables are stripped if necessary. Then you KNOW that every variable you’re dealing in the script doesn’t have backslashes and you have to escape them before posting to db.

1skydive: addslashes() is good for some databases, but others don’t use backslash to escape certain characters. For mysql, it’s better to use mysql_real_escape_string().

Regards

For apache users you can use mod_security and use some rules to protect against sql injection.

I installed it today, broke some scripts but after viewing the audit log was back in in seconds.

Silly

yes, just use mysql_real_escape_string() (as stated above) instead of addslashes. Just note that using the code posted above (includeing it at the top of your pages dealing with gpc) will virtually disable magic_quotes_gpc. It’s not ressources intensive.

:slight_smile:

So you’re saying I should put all incoming vars through this:


function strip_magic_quotes($arr)
{
    foreach ($arr as $k => $v)
    {
        if (is_array($v))
            { $arr[$k] = strip_magic_quotes($v); }
        else
            { $arr[$k] = stripslashes($v); }
    }

    return $arr;
}

if (get_magic_quotes_gpc())
{
    if (!empty($_GET))    { $_GET    = strip_magic_quotes($_GET);    }
    if (!empty($_POST))   { $_POST   = strip_magic_quotes($_POST);   }
    if (!empty($_COOKIE)) { $_COOKIE = strip_magic_quotes($_COOKIE); }
}

and then addslash any that are being used to build a query?

Hmm…makes sense…extra work…lol…but makes sense.

I wish when I wake up tomorrow magic_quotes_* and addslashes() will magically vanish from this world so nobody will have to be confused why they ever existed in the first place. They’re evil. They should die. Escaping (and by all means also quoting) is strictly a database-dependent and context-dependent (like escaping inside a REGEXP or LIKE pattern) issue, and if these “magic” stuff aren’t that intelligently magic they merely just add to the confusion and do no good.

I wonder why PHP developers didn’t also add magic_htmlspecialchars to prevent cross-site scripting? And also magic_preg_quote to prevent that nasty preg ‘/e’ modifier injection that got popular in recently old phpBB scripts? And also magic_urlencode? I guess PHP developers aren’t too creative IMHO in the “magical” sense. :wink:

CapitalWebHost: almost, but DON’T use addslashes. Use mysql_real_escape_string() if you’re using MySQL.

not that much extra work :slight_smile:

just include this recursive function at the top of all your pages dealing with gpc and you’re done.

Also, do not just use mysql_real_escape_string() but also check the expected lengths data type of your variables. See the functions I posted a few posts above as they might save you some time.

:slight_smile:

Got it…thanks. Now to go back and rework all my CMS backend code…:frowning:

LOL.

Better now then later :slight_smile:

someone care to explain what that function do?

What are your methods for preventing sql injection? For example, say I am adding $value into my database. What is the best way to make sure that if $value contains " ’ " it is inserted itno the database, but does not break the query?
I didn’t notice anyone mention filtering or the database specific escaping functions. I would recommend:

// or for string values
$value = preg_replace('/[^a-zA-Z0-9\\_\\-]/', '', $value);
// or for integer values
$value = intval($value);

// and for mysql for example
$sql = "UPDATE mytable SET myfield='" . mysql_real_escape_string($value) . "' WHERE ...";
// or postgres
$sql = "UPDATE mytable SET myfield='" . pg_escape_string($value) . "' WHERE ...";

I believe it is a best practice to filter all values that your script uses from the request and to escape all data that goes into a database.

What if you want to keep whitespace characters?

–ed

What if you want to keep whitespace characters?
Then add any of the them to the regex you want, or use some of the special regex character groups.

 $value = preg_replace('/[^a-zA-Z0-9\\ \\_\\-\\$\\\\\\'\\"]/', '', $value);

The regex is saying: replace all characters not in my set of characters to null.

OK, I just thought there might be a reason why you were filtering out whitespace characters.

–ed

why quoted integers would be a good protection against injections attacks?

“I didn’t notice anyone mention filtering or the database specific escaping functions. I would recommend:”

There it goes again… this thread is now been soooooooo long and filled up with redundant and many disinformation. I would never recommend making up your own escaping function. It’s very easy to make mistakes and that means you may be vulnerable to attacks. And there’s no such thing as database-independent escaping, so even if your function works on MySQL there’s no guarantee it’ll work on other DBMSs.

Why is this “not notice anyone mentioning database-specific escaping function”? Hasn’t you seen mysql_escape_string() or mysql_real_escape_string() being mentioned NUMEROUSLY in this thread? It’s clearly database specific to me, except if you didn’t read the ‘mysql’ part.

My recommendation goes as this: Use a good database abstraction layer, like MDB (my way), Creole (if you have PHP 5), ADOdb, or PEAR DB (last and least of all, but most supported). Then use it’s database-independent escaping function, i.e. quote() for MDB. Rest assured you’ll be safe (unless there’s a bug with the library you’re using).
And please always note that you’ll need to perform additional escaping if you use some data in a LIKE / REGEXP pattern or something else.

I have to warn you that a lot of recommendations in all other posts in this thread won’t prevent SQL injection attacks. And some aren’t portable. A correction to my recommendation is gladly acceptable.