bahaa Posted May 27, 2011 Share Posted May 27, 2011 Hello, Is this function good to use to prevent SQL injection ? function sanitize_values($value) { $new_php = function_exists("mysql_real_escape_string"); $magic_quotes_active = get_magic_quotes_gpc(); if($new_php){ if($magic_quotes_active){$value = stripslashes($value);} $value = mysql_real_escape_string($value); }else { if(!$magic_quotes_active){$value = addslashes($value);} } return $value; } Link to comment Share on other sites More sharing options...
HartleySan Posted May 27, 2011 Share Posted May 27, 2011 Well, I think Larry is the best one to answer this question, but if I recall correctly, there are three main steps to testing input data: 1) Test whether a value is even set. 2) Use regexes to ensure that the value is of the form you expect. 3) Use mysqli_real_escape_string. Anyway, I would definitely like to hear Larry's input on this one, too. 1 Link to comment Share on other sites More sharing options...
bahaa Posted May 27, 2011 Author Share Posted May 27, 2011 Well, I think Larry is the best one to answer this question, but if I recall correctly, there are three main steps to testing input data: 1) Test whether a value is even set. 2) Use regexes to ensure that the value is of the form you expect. 3) Use mysqli_real_escape_string. Anyway, I would definitely like to hear Larry's input on this one, too. I know that there are other steps to validate data, but this one to be used instead of only using the mysqli_real_escape_string because older version of php does not have this function and if there is a new php that has it ,then we should test to see if the magic quotes function on, and if it is on the we have to remove the slashes, so we don't have double slashes. (magic_quotes_gpc(), add_slashes, mysqli_real_escape_string are almost do the same jobe) so basically this function first check check if there is a new version of php that have the mysqli_real_escape_string and also check for the magic quotes function. if there is new php version then we make sure we remove slashes that were applied by magic quotes function. if no new php installed on the server, then we test to see if the magic quotes function is on. if it is not one then we add slashes. Link to comment Share on other sites More sharing options...
HartleySan Posted May 27, 2011 Share Posted May 27, 2011 You may have already seen this page, but I found the following: http://www.tech-evangelist.com/2007/11/05/preventing-sql-injection-attack/ Also, I find it hard to imagine a situation in which you don't know (or can't quickly determine) what version of PHP you're using, and you're concerned that the version might be so old that it does not support mysqli_real_escape_string. Is that a legimate concern? Well, all the same, I think you're on the right track. Edit: And one more good (and detailed) link for good measure: http://php.net/manual/en/security.database.sql-injection.php 1 Link to comment Share on other sites More sharing options...
bahaa Posted May 27, 2011 Author Share Posted May 27, 2011 You may have already seen this page, but I found the following: http://www.tech-evangelist.com/2007/11/05/preventing-sql-injection-attack/ Also, I find it hard to imagine a situation in which you don't know (or can't quickly determine) what version of PHP you're using, and you're concerned that the version might be so old that it does not support mysqli_real_escape_string. Is that a legimate concern? Well, all the same, I think you're on the right track. Edit: And one more good (and detailed) link for good measure: http://php.net/manual/en/security.database.sql-injection.php Well, I think it is better to take all possibilities when you develop even if you know what PHP version will be using. you might move your site from one server to another, or you are developing for some one else and he or she could move the site to another server too. Link to comment Share on other sites More sharing options...
Larry Posted May 27, 2011 Share Posted May 27, 2011 The code you've posted isn't very good, in my opinion. It's a bit hard to read, but... - It's a stylistic issue, but it'd be clearer if the function_exists() and get_magic_quotes_gpc() calls were the conditions themselves, instead of assigning their calls to variables and then using the variables. - mysql_real_escape_string() is by no means "new". It was added in PHP 4.3. - It'd be better if the function tested for the existence of mysqli_real_escape_string() and used that, or mysql_real_escape_string() instead. - The *_real_escape_string() functions require a database connection, so one ought to be made available to the function. - addslashes() isn't nearly as secure as the *_real_escape_string() functions. If the code can't even use mysql_real_escape_string(), then prepared statements or other solutions ought to be applied instead. Link to comment Share on other sites More sharing options...
bahaa Posted May 27, 2011 Author Share Posted May 27, 2011 Is this better ? function sanitize_values($value) { // Check of the Mysqli_real_escape_string function is available if(function_exists("mysqli_real_escape_string")){ // If it is available, then we check if the magic quotes function is on if(get_magic_quotes_gpc()){$value = stripslashes($value);} // If magic quotes function is on, we remove any slashes applied by magic quotes $value = mysqli_real_escape_string($value); } else { if(!get_magic_quotes_gpc()){$value = addslashes($value);} } return $value; } Link to comment Share on other sites More sharing options...
Antonio Conte Posted May 27, 2011 Share Posted May 27, 2011 As Larry point out, why code for such an old version of PHP? It just don't make any sense. I would check for mysqli-extension. If it does exsist, use prepared statement, otherwise, use mysql_real_escape_string and other checks. Link to comment Share on other sites More sharing options...
bahaa Posted May 27, 2011 Author Share Posted May 27, 2011 As Larry point out, why code for such an old version of PHP? It just don't make any sense. I would check for mysqli-extension. If it does exsist, use prepared statement, otherwise, use mysql_real_escape_string and other checks. Even if you use mysql_real_escape_string, you have to check if the magic_quotes_gpc() is on or not. I have a hosting with godaddy and they have magic_quotes_gpc() set to on, so if I want to use mysql_real_escape_string then I would have to remove any slashes that were added by magic_quotes If magic_quotes_gpc is enabled, first apply stripslashes() to the data. Using this function on data which has already been escaped will escape the data twice. http://php.net/manual/en/function.mysql-real-escape-string.php 1 Link to comment Share on other sites More sharing options...
Antonio Conte Posted May 28, 2011 Share Posted May 28, 2011 You don't need mysql*_real_escape_string() with Prepared Statement. I absolutely see your point though // Set Mysqli or Mysqli $MySQLI == TRUE; if ($MySQLI == true) { // do Prepared statements } else { self::sanitize_values($input); // do things with $value } private function sanitize_values($value) { // Only remove if magic quotes are enabled if ( get_magic_quotes_gpc() ) { $value = stripslashes($value); } // Then use mysql*_real_escape_string $value = mysql_real_escape_string($value); return $value; } 1 Link to comment Share on other sites More sharing options...
Recommended Posts