Page 1 of 1

Will this protect from SQL injection?

Posted: Thu Nov 06, 2008 5:23 am
by Sindarin
When the user submits the form I replace the following characters (I use wysiwyg editor),
$string = str_replace("'",":apos;",$string);
$string = str_replace("`",":apos;",$string);
$string = str_replace('"',":quote;",$string);
$string = str_replace("%",":percnt;",$string);
$string = str_replace("\",":backsl;",$string);
$string = str_replace("/",":slash;",$string);
$string = str_replace("|",":macro;",$string);
$string = str_replace("*",":wildc;",$string);
$string = str_replace("$",":dollar;",$string);
$string = str_replace("<",":ltag;",$string);
$string = str_replace(">",":rtag;",$string);
$string = str_replace(";",":lineb;",$string);
$string = str_replace(",",":comma;",$string);
and when I display the entries, I replace back the characters so nothing gets erased, e.g. :backsl; becomes \

Are there any holes to this method?

Re: Will this protect from SQL injection?

Posted: Thu Nov 06, 2008 5:33 am
by Mordred
Will this protect from SQL injection?
No.

Reading this might help:
http://www.logris.org/security/the-unex ... -injection

Re: Will this protect from SQL injection?

Posted: Thu Nov 06, 2008 5:45 am
by Sindarin
mysql_real_escape_string() does not work on my host. magic_quotes are on so I have to do it myself. Can you elaborate on what I am doing wrong in the above code?

Re: Will this protect from SQL injection?

Posted: Thu Nov 06, 2008 5:49 am
by papa
Read Example #3. You can probably use something similar to this:

Code: Select all

 
 if(get_magic_quotes_gpc()) {
            $product_name        = stripslashes($_POST['product_name']);
            $product_description = stripslashes($_POST['product_description']);
        } else {
            $product_name        = $_POST['product_name'];
            $product_description = $_POST['product_description'];
        }
 
http://us.php.net/manual/ro/function.my ... string.php

Re: Will this protect from SQL injection?

Posted: Thu Nov 06, 2008 5:59 am
by Sindarin
stripslashes
doesn't it just remove the slashes? What is more than just using str_replace/ereg to do it manually for all the illegal characters?
And what is more to mysql_real_escape_string() ?

Re: Will this protect from SQL injection?

Posted: Thu Nov 06, 2008 6:01 am
by Eran
magic_quotes can also be set using ini_set() or through .htaccess, so it's very easy to turn off. Also, if the mysql extension is installed mysql_real_escape_string will be available. Make sure you are creating a database connection using mysql_connect, since it is needed for running mysql_real_escape_string.

Re: Will this protect from SQL injection?

Posted: Thu Nov 06, 2008 6:02 am
by papa
As I said, read the example: :)

Code: Select all

 
<?php
 
if (isset($_POST['product_name']) && isset($_POST['product_description']) && isset($_POST['user_id'])) {
    // Connect
 
    $link = mysql_connect('mysql_host', 'mysql_user', 'mysql_password');
 
    if(!is_resource($link)) {
 
        echo "Failed to connect to the server\n";
        // ... log the error properly
 
    } else {
        
        // Reverse magic_quotes_gpc/magic_quotes_sybase effects on those vars if ON.
 
        if(get_magic_quotes_gpc()) {
            $product_name        = stripslashes($_POST['product_name']);
            $product_description = stripslashes($_POST['product_description']);
        } else {
            $product_name        = $_POST['product_name'];
            $product_description = $_POST['product_description'];
        }
 
        // Make a safe query
        $query = sprintf("INSERT INTO products (`name`, `description`, `user_id`) VALUES ('%s', '%s', %d)",
                    mysql_real_escape_string($product_name, $link),
                    mysql_real_escape_string($product_description, $link),
                    $_POST['user_id']);
 
        mysql_query($query, $link);
 
        if (mysql_affected_rows($link) > 0) {
            echo "Product inserted\n";
        }
    }
} else {
    echo "Fill the form properly\n";
}
?>
 
 

Re: Will this protect from SQL injection?

Posted: Thu Nov 06, 2008 6:05 am
by Sindarin
I also found in the doc,
\x00, \n, \r, \, ', " and \x1a
Are those the only characters that can be used in an injection?

I just can't understand why mysql_real_escape_string() is considered best practice than doing it manually. In essence it just removes those characters while my method allows the necessary characters like quote or apostrophe to be encoded and decoded without harming the query therefore with no loss in content.

Re: Will this protect from SQL injection?

Posted: Thu Nov 06, 2008 6:31 am
by Eran
First of all, it doesn't remove the characters - it only escapes them. Second, since it is a native function and compiled in C, it runs much faster than your multiple search and replace functions. Third, it is aware of the character encoding of the database connection. And fourth, binary data can only escaped with this function.

Why shouldn't you use it?

Re: Will this protect from SQL injection?

Posted: Fri Nov 07, 2008 4:34 am
by Sindarin
First of all, it doesn't remove the characters - it only escapes them. Second, since it is a native function and compiled in C, it runs much faster than your multiple search and replace functions. Third, it is aware of the character encoding of the database connection. And fourth, binary data can only escaped with this function.
Okay, this answer seems sufficient. Thanks.

Re: Will this protect from SQL injection?

Posted: Fri Nov 21, 2008 5:37 am
by Sindarin
Another question would be is there any way to make it more compact than below?

Code: Select all

 
$article_title=mysql_real_escape_string($article_title,$db_connection);
$article_content=mysql_real_escape_string($article_content,$db_connection);
$article_tags=mysql_real_escape_string($article_tags,$db_connection);
$article_date=mysql_real_escape_string($article_date,$db_connection);
$article_time=mysql_real_escape_string($article_time,$db_connection);
$article_shownews=mysql_real_escape_string($article_shownews,$db_connection);
$article_expirehome=mysql_real_escape_string($article_expirehome,$db_connection);
$article_previewimage=mysql_real_escape_string($article_previewimage,$db_connection);
$article_contentimage=mysql_real_escape_string($article_contentimage,$db_connection);
$article_redirect=mysql_real_escape_string($article_redirect,$db_connection);
$article_showhome=mysql_real_escape_string($article_showhome,$db_connection);
$article_importance=mysql_real_escape_string($article_importance,$db_connection);
$article_showrss=mysql_real_escape_string($article_showrss,$db_connection);
$article_expire_date=mysql_real_escape_string($article_expire_date,$db_connection);
$article_expire_time=mysql_real_escape_string($article_expire_time,$db_connection);
and finally, will the above protect against sql injection or do I have to do anything else?

Re: Will this protect from SQL injection?

Posted: Fri Nov 21, 2008 5:55 am
by papa
Loop through your query.

My not so perfect example maybe can give you some ideas:

mySQLInsert("value1,value2", "value_table");

Code: Select all

 
    function mySQLInsert($values, $mySQLtable) {
        //DB connect
        $link = $this->dbConnect();
        //assoc arrays
        $cols = implode(",", array_keys($values));
        $vals = array_values($values);
        //Create MySQL query string
        foreach($vals as $val) {
            if(get_magic_quotes_gpc()) {
                $val = stripslashes($val);
            }
            //MySQL escape
            $query[] = mysql_real_escape_string($val, $link);
            switch($val) {
                case "NULL":
                $placeholder[] = "NULL";
                break;
                case is_int($val):
                $placeholder[] = "'%d'";
                break;
                default:
                $placeholder[] = "'%s'";
            }
        }
        $sql_values = implode(", ", $placeholder);
        //MySQL query
        $query = array_merge(array("INSERT INTO ".$mySQLtable." ({$cols}) VALUES ({$sql_values})"), $query);
        $query = call_user_func_array("sprintf", $query);
        if(!$result = @mysql_query($query)) {
            $this->errorMSG[] = "Query failed: " . mysql_error();
            return false;
            exit;   
        } else {
        return mysql_insert_id();
        } 
    }

Re: Will this protect from SQL injection?

Posted: Sun Nov 23, 2008 11:32 pm
by alex.barylski
Honestly just use PDO it intrinsically protects against SQL injection by both:

1. Escaping according to the driver you have selected
2. Automatically casting types

The latter I have confirmed by just testing passing it a PKID as a string and it still works so I assume PDO has the insight to know that field A is an integer and will convert accordingly. You also do not need to wrap string parameters in quotes so this backs up the claim that it uses native DB field type meta data to properly cast data.

Re: Will this protect from SQL injection?

Posted: Fri Nov 28, 2008 12:25 pm
by John Cartwright
Typically you would create an array to group the related data.

Code: Select all

$article['title'] = 'foobar';
$article['content'] = 'this is the content';
$article['date'] = '01/01/2001';
 
//callback function
$article = array_map('mysql_real_escape_string', $article); 
 
//or a loop by reference
foreach ($article as &$value) {
   $value = mysql_real_escape_string($value, $db_connection);
}
 
//or loop by key
foreach ($article as $key => $value) {
   $article[$key] = mysql_real_escape_string($value, $db_connection);
}

Re: Will this protect from SQL injection?

Posted: Mon Dec 01, 2008 4:08 am
by Sindarin
Ah, arrays. They always confuse me, but I have to learn them as they seem really essential.