waynewex Posted June 21, 2008 Share Posted June 21, 2008 I've just started working on a function to stop or make SQL injection more difficult. This is intended to clean up forum posts. Any criticisms or pointers are welcome. Thanks. <?php //clean_user_post is to be used for all user comments, forum posts etc. function clean_user_post($message){ //checks for common sql injection strings. if(stristr($message, "DROP TABLE") || stristr($message, "DESCRIBE TABLE") || stristr($message, "SELECT *") || stristr($message, "OR 1 = 1")){ return false; //if possible inject string found } if(stristr($message, "-")){ $message = eregi_replace("-","~",$message); } $message = addslashes($message); $message = htmlentities($message); return $message; } ?> Quote Link to comment Share on other sites More sharing options...
hitman6003 Posted June 21, 2008 Share Posted June 21, 2008 if(stristr($message, "DROP TABLE") || stristr($message, "DESCRIBE TABLE") || stristr($message, "SELECT *") || stristr($message, "OR 1 = 1")){ return false; //if possible inject string found } You just eliminated anyone wanting to use it for posing SQL code, as we do here. Quote Link to comment Share on other sites More sharing options...
waynewex Posted June 21, 2008 Author Share Posted June 21, 2008 Yea, I know, I've been crtitical of that part of the code mysql: //checks for common sql injection strings. if(stristr($message, "DROP TABLE") || stristr($message, "DESCRIBE TABLE") || stristr($message, "SELECT *") || stristr($message, "OR 1 = 1")){ return false; //if possible inject string found } Maybe I'll only use that on password and username forms. So, would the rest, coupled with usage of sprintf for query building, be effective? Quote Link to comment Share on other sites More sharing options...
hitman6003 Posted June 21, 2008 Share Posted June 21, 2008 The most effective method of preventing SQL injection is to use the db's own escape method... http://www.php.net/mysql_real_escape_string Or, if you're using PDO (which I prefer), then use the prepare method on each statement. Quote Link to comment Share on other sites More sharing options...
waynewex Posted June 21, 2008 Author Share Posted June 21, 2008 Is there a reason why I can't echo the $message after using this function: <?php //clean_user_post is to be used for all user comments, forum posts etc. function clean_user_post($message){ if(stristr($message, "-")){ $message = eregi_replace("-","~",$message); } if(get_magic_quotes_gpc){ $message = stripslashes($message); } $message = mysql_real_escape_string($message, $connection); $message = addslashes($message); $message = htmlentities($message); return $message; } ?> Quote Link to comment Share on other sites More sharing options...
DarkWater Posted June 21, 2008 Share Posted June 21, 2008 Well, are you doing something like: $message = $_POST['post']; $message = clean_user_post($message); That's how you should be doing it. Quote Link to comment Share on other sites More sharing options...
waynewex Posted June 21, 2008 Author Share Posted June 21, 2008 I am doing that on another test page: if(isset($_POST['hiddenField'])){ include("file.php"); $test = $_POST['user']; $test = clean_user_post($test); } Quote Link to comment Share on other sites More sharing options...
DarkWater Posted June 21, 2008 Share Posted June 21, 2008 Change: $message = mysql_real_escape_string($message, $connection); To: $message = mysql_real_escape_string($message); You didn't make the connection a global var. Don't know why it isn't yelling at you, honestly. o-O Also, change the eregi_replace to str_replace. It's faster. Quote Link to comment Share on other sites More sharing options...
waynewex Posted June 21, 2008 Author Share Posted June 21, 2008 Code is as follows: On test page(testing the function): <?php if(isset($_POST['hiddenField'])){ include("file.php"); $test = $_POST['user']; $test = clean_user_post($test); } ?> <HTML> <HEAD> <TITLE>Test</TITLE> </HEAD> <BODY> <?php echo $test; ?> <form name="form1" method="post" action=""> <p> <input type="text" name="user" id="user"> </p> <p> <input type="submit" name="button" id="button" value="Submit"> <input type="hidden" name="hiddenField" id="hiddenField"> </p> </form> </BODY> </HTML> Then on the functions page: <?php //clean_user_post is to be used for all user comments, forum posts etc. function clean_user_post($message){ if(stristr($message, "-")){ $message = str_replace("-","~",$message); } if(get_magic_quotes_gpc){ //independent of magic quotes. $message = stripslashes($message); } $message = mysql_real_escape_string($message); $message = htmlentities($message); return $message; } ?> Quote Link to comment Share on other sites More sharing options...
DarkWater Posted June 21, 2008 Share Posted June 21, 2008 First of all: You need to change: if(get_magic_quotes_gpc){ To: if(get_magic_quotes_gpc()){ Secondly, "file.php" is what contains the functions, correct? Quote Link to comment 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.