s0c0 Posted February 5, 2009 Share Posted February 5, 2009 Can someone please validate for me that this method will successfully prevent SQL injection attempts. I would just like a second set of eyes before implementing this. public function returnSafeSQL($data,$strict=true) { // strict mode - removes: virtually all non-alphanumeric characters,adds slashes,DROP TABLE,DELETE FROM,ALTER TABLE,INSERT INTO if($strict===true) { $evilSQLArr = array('/DROP TABLE/','/DELETE FROM/','/ALTER TABLE/','/INSERT INTO/','/SELECT/','/=/','/--/','/;/'); if(is_array($data)) // one dimensional { foreach($data as $array_index=>$variable) { if(is_array($variable)) // two dimensional { foreach($variable as $index=>$var) { if(is_array($var)) // three dimensional { foreach($var as $i=>$v) { $var[$i]=strip_tags($v); $var[$i]=preg_replace($evilSQLArr,'',$v); $var[$i]=ereg_replace("[^[:space:]a-zA-Z0-9_.,@!$:~']", "", $v); $var[$i]=addslashes($v); } } else { $variable[$index]=strip_tags($var); $variable[$index]=preg_replace($evilSQLArr,'',$var); $variable[$index]=ereg_replace("[^[:space:]a-zA-Z0-9_.,@!$:~']", "", $var); $variable[$index]=addslashes($var); } } } else // single dimensional { $data[$array_index]=strip_tags($variable); $data[$array_index]=preg_replace($evilSQLArr,'',$variable); $data[$array_index]=ereg_replace("[^[:space:]a-zA-Z0-9_.,@!$:~']", "", $variable); $data[$array_index]=addslashes($variable); } } } else // non-array, just a string { $data = strip_tags($data); $data = preg_replace($evilSQLArr,'',$data); $data = ereg_replace("[^[:space:]a-zA-Z0-9_.,@!$:~']", "", $data); $data = addslashes($data); } } else // just adds slashes for non-strict mode { if(is_array($data)) // one dimensional { foreach($data as $array_index=>$variable) { if(is_array($variable)) // two dimensional { foreach($variable as $index=>$var) { if(is_array($var)) // three dimensional { foreach($var as $i=>$v) { $var[$i]=addslashes($v); } } else { $variable[$index]=addslashes($var); } } } else { $data[$array_index]=addslashes($variable); } } } else // non-array, just a string { $data = addslashes($data); } } return $data; } Quote Link to comment https://forums.phpfreaks.com/topic/143934-please-validate-this-sql-injection-prevention-method/ Share on other sites More sharing options...
killah Posted February 5, 2009 Share Posted February 5, 2009 I have not slept in 36 +- hour's due to big job needing to be done. So i am currently half asleep. I never went through your hole code. But for one i know about sql injection's is. the attacker some time's uses a UNION ALL SELECT So maybe forbding UNION ALL SELECT. On another note. Since i never really went through your code i did not go as far as to your regex code. Maybe add /i to make it case insensitive incase attacker's mess around and put UnIoN all SeLECT. It has happened on a site of mine where the attacker did that. But stupidity of me and being new to php at that given time i never went to REGEX Checking and went with simple str_replace. For this str_replace does not work. I have a nice little database class that i just created 2 week's ago for my RPG engine. It work's like sprintf but in a way faster than sprintf. My database class is also very fast and ran up to 7,000 query's in less than a second. If i am not mistaken it was 0.09176 milli second's to run 7k query's. Which in my eyes is very good to run and this being put on a layout of plain image's. So it took about 8 image's to load & 7k query's in less than a second. Thus being on shared hosting also proved to me how www.byethost.com really is good. Let me stop now with my lecture and say Add union all select if not make them params union all select they are very comon in sql injection's and also 1=1. Hope this help's in any way. Quote Link to comment https://forums.phpfreaks.com/topic/143934-please-validate-this-sql-injection-prevention-method/#findComment-755271 Share on other sites More sharing options...
JonnoTheDev Posted February 5, 2009 Share Posted February 5, 2009 mysql_real_escape_string() on its own would have done the job. Also make sure that you database user only has the privileges that are required. http://uk3.php.net/manual/en/function.mysql-real-escape-string.php Quote Link to comment https://forums.phpfreaks.com/topic/143934-please-validate-this-sql-injection-prevention-method/#findComment-755276 Share on other sites More sharing options...
s0c0 Posted February 5, 2009 Author Share Posted February 5, 2009 killah, thanks get some rest. neil, should i use both mysql_real_escape_string() and addslashes or just mysql_real_escape_string(). I believe they do the same thing, correct? Quote Link to comment https://forums.phpfreaks.com/topic/143934-please-validate-this-sql-injection-prevention-method/#findComment-755279 Share on other sites More sharing options...
killah Posted February 5, 2009 Share Posted February 5, 2009 addslashes add's \ infront of all the ' on other hand. mres does more. Quote Link to comment https://forums.phpfreaks.com/topic/143934-please-validate-this-sql-injection-prevention-method/#findComment-755282 Share on other sites More sharing options...
JonnoTheDev Posted February 5, 2009 Share Posted February 5, 2009 They dont do the same thing entirely. mysql_real_escape_string() escapes special characters used in queries but will not store the slashes in your db row. Forget addslashes() it is a poor function in my opinion and requires the use of stripslashes() when using output. If you want to be even more secure switch to the mysqli extensions and use parameterized querys: <?php $connection = new mysqli("localhost","user","pass","db"); $query = $connection->prepare("SELECT * FROM table WHERE id = ?"); $query->bind_param("i", $id); $query->execute(); ?> Running your function above through every input value is going to slow things down. Quote Link to comment https://forums.phpfreaks.com/topic/143934-please-validate-this-sql-injection-prevention-method/#findComment-755284 Share on other sites More sharing options...
killah Posted February 5, 2009 Share Posted February 5, 2009 Why change to mysqli when using sprintf can do that for you? Not trying to say not to use mysqli because on my site i run some functions that are included in mysqli and are very good. But if you are not used to it rather don't change till you know the class. $query = sprintf('SELECT * FROM `table` WHERE `id` = %d', abs(@intval($ID))); mysql_query($query); given that we use abs(@intval($ID)) to make sure it's a number and only a number. www.php.net/sprintf www.php.net/printf Quote Link to comment https://forums.phpfreaks.com/topic/143934-please-validate-this-sql-injection-prevention-method/#findComment-755290 Share on other sites More sharing options...
JonnoTheDev Posted February 5, 2009 Share Posted February 5, 2009 Up to you but as to why: More up to date Improved functionality (the i stands for improved) Allows you to use the functions provided in MYSQL 4.1 and above Object orientated interface and so on Quote Link to comment https://forums.phpfreaks.com/topic/143934-please-validate-this-sql-injection-prevention-method/#findComment-755300 Share on other sites More sharing options...
killah Posted February 5, 2009 Share Posted February 5, 2009 I have used MySQL Improved before. But as i said. If you are not used to there clases and how it function's. Don't change over as off yet. Best bet is to use INNOB and mysqli for safe transaction. But again. If not used to it don't change over to it. Quote Link to comment https://forums.phpfreaks.com/topic/143934-please-validate-this-sql-injection-prevention-method/#findComment-755304 Share on other sites More sharing options...
s0c0 Posted February 5, 2009 Author Share Posted February 5, 2009 I do use mysqli and innodb, however I am not using it in much of an object oriented manner. $this->LINK = mysqli_connect('localhost','user','pass',$db_name); $this->CONNECTION = mysqli_select_db($this->LINK,$db_name); when i run a query i then: $result = mysqli_query($this->LINK,$sql); Quote Link to comment https://forums.phpfreaks.com/topic/143934-please-validate-this-sql-injection-prevention-method/#findComment-755383 Share on other sites More sharing options...
JonnoTheDev Posted February 6, 2009 Share Posted February 6, 2009 yes you are you just aren't realising it. $this-> is a property/method reference within an object $result = mysqli_query($this->LINK,$sql); Quote Link to comment https://forums.phpfreaks.com/topic/143934-please-validate-this-sql-injection-prevention-method/#findComment-755885 Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.