Tell me more ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.
up vote 3 down vote favorite
2

I'm trying to defend the sign-in form on my page from SQL injections. On the serverside, I use Zend Framework (Zend_Db,Zend_Db_Table_Abstract), but its build-in injection prevention functions: quote, quoteInto, quoteIdentifier don't make their work well (as far as I know how to use them). Other ways like mysql_real_escape_string, addslashes do not seem to work at all...

This is what I'm trying to implement for the defense:

function prevent_from_sql_injection($str) {
    if(preg_match('/[\'"]/', $str))
     {die('attack1'); exit;  }// no quotes
elseif(preg_match('/[\/\\\\]/', $str))
     {die('attack2'); exit;  }// no slashes
elseif(preg_match('/(and|or|null|not)/i', $str))
     {die('attack3'); exit;  }// no sqli boolean keywords
elseif(preg_match('/(union|select|from|where)/i', $str))
     {die('attack4'); exit;  }// no sqli select keywords
elseif(preg_match('/(group|order|having|limit)/i', $str))
     {die('attack5'); exit;  }//  no sqli select keywords
elseif(preg_match('/(into|file|case|LOAD_FILE|DUMPFILE|char|schema|AES_DECRYPT|AES_ENCRYPT)/i', $str))
     {die('attack6'); exit;  }// no sqli operators
elseif(preg_match('/(--|#|\/\*)/', $str))
     {die('attack7'); exit; }// no sqli comments
elseif(preg_match('/(=|&|\|)/', $str))
     {die('attack8'); exit;  }// no boolean operators
elseif(preg_match('/(UNI\*\*ON|1 OR 1=1|1 AND 1=1|1 EXEC XP_)/', $str))
     {die('attack9'); exit; }
elseif(preg_match('/(1|'| |O|R|=|&#49&#39&#32&#79&#82&#32&#39&#49&#39&#61&#39&#49|%31%27%20%4F%52%20%27%31%27%3D%27%31)/', $str))
     { die('attack10'); exit; }
elseif(preg_match('/(SELECT\s[\w\*\)\(\,\s]+\sFROM\s[\w]+)| (UPDATE\s[\w]+\sSET\s[\w\,\'\=]+)| (INSERT\sINTO\s[\d\w]+[\s\w\d\)\(\,]*\sVALUES\s\([\d\w\'\,\)]+)| (DELETE\sFROM\s[\d\w\'\=]+)/', $str))
     { die('attack11'); exit; } 
elseif(preg_match('/(script)|(<)|(>)|(%3c)|(%3e)|(SELECT) |(UPDATE) |(INSERT) |(DELETE)|(GRANT) |(REVOKE)|(UNION)|(<)|(>)/', $str))
     { die('attack12'); exit; } 
elseif(!preg_match('/^["a-zA-Z0-9\040]+$/', $str))
     { die('attack13'); exit; } 
else return $str;

}

As to test my results, I use Firefox extension SQL Inject Me and it shows 14 more errors (sometimes 21 or 17 and I don't know why the results are different):

Server Status Code: 302 Found
Tested value: 1' OR '1'='1
Server Status Code: 302 Found
Tested value: 1 UNI/**/ON SELECT ALL FROM WHERE
Server Status Code: 302 Found
Tested value: &#49&#39&#32&#79&#82&#32&#39&#49&#39&#61&#39&#49
Server Status Code: 302 Found
Tested value: 1 OR 1=1
Server Status Code: 302 Found
Tested value: 1' OR '1'='1
Server Status Code: 302 Found
Tested value: 1 EXEC XP_
Server Status Code: 302 Found
Tested value: 1 UNION ALL SELECT 1,2,3,4,5,6,name FROM sysObjects WHERE xtype = 'U' --
Server Status Code: 302 Found
Tested value: %31%27%20%4F%52%20%27%31%27%3D%27%31
Server Status Code: 302 Found
Tested value: 1 AND 1=1
Server Status Code: 302 Found
Tested value: 1' OR '1'='1
Server Status Code: 302 Found
Tested value: 1 AND ASCII(LOWER(SUBSTRING((SELECT TOP 1 name FROM sysobjects WHERE xtype='U'), 1, 1))) > 116

So what is the best way to prevent all this SQL injection attacks? Using placeholders is good but it doesn't ok in some cases. Maybe this extension is wrong and I have a paranoia?

share|improve this question
Define how the other options you mentioned "don't work well". – Daniel Egeberg Aug 1 '10 at 20:27
4  
Have you considered some of these are just flase reports? Just because server returns 302 code,does not mean that injection succeeded. – Mchl Aug 1 '10 at 20:35
Maybe you're right and this ext returns incorrect results. But how do then I test for sql injection? – user406136 Aug 2 '10 at 7:15
Seeing such a ridiculous code snippets, I am always wondering, what if a site, where it have been posted, used the same "protection" method? How could you post a elseif(preg_match('/(and|or|null|not)/i', $str)) string if there was such a checking here on so? – Your Common Sense Aug 3 '10 at 8:24
but its build-in injection prevention functions: quote, quoteInto, quoteIdentifier don't make their work well LOL. It works for everyone. So, the problem is somewhere else. Somewhere between the chair and the monitor... – Your Common Sense Aug 3 '10 at 8:27
show 1 more comment

3 Answers

up vote 5 down vote accepted

I strongly recommend the use of Zend_DB. It uses prepared statements.
The parameters to prepared statements don't need to be quoted; the driver automatically handles this.

If an application exclusively uses prepared statements, the developer can be sure that no SQL injection will occur (however, if other portions of the query are being built up with unescaped input, SQL injection is still possible

$db = Zend_Db::factory('Pdo_Mysql', array(
    'host'     => '127.0.0.1',
    'username' => 'webuser',
    'password' => 'xxxxxxxx',
    'dbname'   => 'test'
));

$stmt = $db->query('SELECT * FROM bugs WHERE reported_by = ? AND bug_status = ?',
    array('goofy', 'FIXED')
);

$rows = $stmt->fetchAll();

echo $rows[0]['bug_description'];
share|improve this answer
These params in array - are they quoted automatically or what? – user406136 Aug 2 '10 at 7:13
1  
Yes. That's what prepared statements are for. "The parameters to prepared statements don't need to be quoted; the driver automatically handles this." Please read the linked source for further information. – Benjamin Cremer Aug 2 '10 at 8:31
I agree with Benjamin, using Zend_Db takes out all the worry of prepared statements and I've not had any issues with it. – Grant Collins Aug 2 '10 at 8:37
Sometimes prepared statements can allow SQL injections as well: en.wikipedia.org/wiki/… – FractalizeR Aug 3 '10 at 8:32
@FractalizeR "if other portions of the query are being built up with unescaped input, SQL injection is still possible" – Benjamin Cremer Aug 3 '10 at 9:35
show 1 more comment
up vote -1 down vote

Put this code into your header page.

function clean_header($string)
{
$string = trim($string);

// From RFC 822: “The field-body may be composed of any ASCII
// characters, except CR or LF.”
if (strpos($string, “\n“) !== false) {
$string = substr($string, 0, strpos($string, “\n“));
}
if (strpos($string, “\r“) !== false) {
$string = substr($string, 0, strpos($string, “\r“));
}

return $string;
}
share|improve this answer
up vote 6 down vote

Use prepared SQL statements instead of value escaping.

 $st = $pdo->prepare("SELECT * FROM tbl WHERE x = ?");
 $st->execute($x);

Blacklisting a couple of SQL statements isn't a sensible approach. That Firefox extension is misleading and gave you the wrong impression.

Obviously you shouldn't start to accept random SQL commands as input. At most you use some request variables as query data. And prepared statements are sufficient to provide security for that case. Concatenating SQL commands and variables is an outdated practice (what that questionable Firefox extension seems to suggest.).

share|improve this answer
And that's exactly what Zend_Db offers. – Mchl Aug 1 '10 at 21:55
What should I do with Zend_Auth_Adapter_DbTable ? – user406136 Aug 2 '10 at 7:12
1  
actually there is no problem with escaping if used properly – Your Common Sense Aug 3 '10 at 8:38

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.