Code Review - SQL and Insertion Attacks (Warning: Not for Newbs)

[R_P]

Hey guys,

 

Its been a while, I know. Use to love coming here to answer peoples questions, but work and school have been keeping me too busy but do anything but lurk. I usually write all my code/apps by hand and over the past couple years created a common set of PHP functions that I implement on every site. This code continues to grow, and may become a framework one day (http://ryan.crawford.com/framework/default/). I wanted to present some of the crux functions for your consideration. Specifically I'd like to know what your best practices are in relation to preventing SQL insertion attacks, looking at the functions I use to control it:

 

private $chars = array(
	";" => "{00sc}", "'" => "{01sq}",
	"!" => "{02ex}", "$" => "{03dl}",
	"%" => "{04pr}", "<" => "{05ls}",
	">" => "{06gt}", "=" => "{07eq}",
	"&" => "{08an}", "#" => "{09pd}",
	"," => "{10cm}", "/" => "{11fs}",
	"*" => "{12as}", "\\"=> "{13bs}"
);

/*
* Func: inject($str) - aptly named 
* Desc: We'll be the only people doing SQL injection here
*/
function inject($str) {
    return str_replace(array_keys($this->chars),
		array_values($this->chars),$str);
}

/*
* Func: extract($str)
* Desc: Opposite of inject
*/
function extract($str) {
	$str = str_replace(array_values($this->depc),
		array_keys($this->depc),$str);

	return str_replace(array_values($this->chars),
		array_keys($this->chars),$str);
}	

/*
* Func: query($query_data)
* Desc: Make a query on the database (SELECT)
* Note: If a log directory is defined, we will track queries
*/
function query($qdata) { 
    $result = mysql_query($qdata) or die("<br>Query: ".$qdata." <br><br>Issue: " . mysql_error());  
    
	// set the condition for the switch statement        
    $c = substr($qdata,0,strpos($qdata,' '));  
    
    if($c == "DELETE"||$c == "INSERT"||$c == "UPDATE") { 
    	if(is_dir($this->cfg['logdir'])) 
    		$this->logLine($qdata,$this->cfg['qlog']);
        return true;
    }
	if(mysql_num_rows($result)==0)
		return false;  
		   
	while($line = mysql_fetch_array($result,MYSQL_ASSOC)) {
		$array_result[]=$this->extract($line);
	}	
	return $array_result;   
}

/*
* Func: iquery($array,$table)
* Desc: Insert data into the db(using just $_POST)
*/
function iquery($arr,$table) {    	
	if(!$dataArr = $this->againstTable($arr,$table))
		return false;	
	$n = 1;
	// Loop to create SQL query
	foreach($dataArr as $key => $value) {
		$insertNames .= (sizeof($dataArr)==$n)? $key : $key.",";
		$insertValues.= (sizeof($dataArr)==$n)? "'".$value."'" : "'".$value."',";
		$n++;
	}
	$this->query("INSERT INTO ".$table." (".$insertNames.") VALUES (".$insertValues.");");
}

 

Basically, if you look at the array $chars, for inserted data I am searching for the keys and converting them to the values before insertion. It is a function I run all my inserts through. The query() function is what I do all my SELECTS with and it converts the characters back (the inject function encrypts, extract decrypts for lack of a better term). These are written object oriented so I still have access to PHP's extract if ever needed.

 

Question now being, what do you think of the effectiveness of this technique in terms of protecting against SQL injection? Are there ways to beat it? Are there better methods? Is this efficient? I'm not sure what algorithm PHP is using for str_replace, but the best ones don't run any faster than O(n). I don't think this runs any worse.

[cags]

To my knowlege mysql_real_escape_string() protects against all possible SQL injection attacks.

[KevinM1]

To my knowlege mysql_real_escape_string() protects against all possible SQL injection attacks.

 

For the MySQL database, yes.  There are also other built-in functions that do the same thing for other brands of db.  Even better, using the MySQLi extension or PDO gives one prepared statements, which automatically escape values.

[PFMaBiSmAd]

Actually, mysql_real_escape_string(), like its' name indicates, only protects against sql injection in string data (i.e. data that is enclosed in quotes in the query.) It does not protect against sql injection being done in numeric data (i.e data that is not enclosed in quotes in the query.) Nor does it protect against sql injection when an identifier (database, table, or column name) in the query comes from an external variable.

[yettti]

I find that protecting against SQL injection is best done accordingly to the data you want, protecting against SQL injection is good, but what you really want to do is sanitize your input.

 

I know what your thinking, there the same thing, but i find that doing things such as extracting parts of a string according to a REGEX expression are alot more efficient, not only do the minimise the risk of SQL injection (mysql_real_escape_string() still being used for safe measures) but they also minimize the possibility for your code to crash.

 

Well, thats my way of doing it, im sure everyones got a different way.