How they works: DB-Connect and SQL Injection?

Question

My question is regarding the application of a DB connection and SQL injection. I am using the codes provided below to successfully connect to my database. I would like to ask:

1. How the foreach stops SQL injection?
2. If there is a better more efficient way to make the connection more secure?
3. Will the connection still work and provide me with valid data from the database, if dbconnect.php were to be included in another file (for example; global.php), which in return was included in the main file where the actual use of database is?

example_file.php:

<?php 
//MySQL Database Connect
include './includes/dbconnect.php'; 


//This stops SQL Injection in POST vars
foreach ($_POST as $key => $value) {
  $_POST[$key] = mysql_real_escape_string($value);
}
//This stops SQL Injection in GET vars
foreach ($_GET as $key => $value) {
  $_GET[$key] = mysql_real_escape_string($value);
}
?>

dbconnect.php:

<?php
$con = mysql_connect("localhost","username","password");

if (!$con){ die('Could not connect: ' . mysql_error()); }
mysql_select_db("databasename", $con);

?>

Thanks a lot!

Answer 1

1. How the foreach stops SQL injection?

   This is really really important, so I'm going to say it very loudly in the hope that people might understand…

   IT DOESN'T!

   "Why not?", I hear you ask. Well, my young padawan, that is because:

   1. It only escapes strings

      Let's imagine a website through which users can view their medical records. To enable the user to filter his records for those related to a particular illness, one permits the illness to be specified by its id as a query parameter:

      http://www.example.com/health/fetchmyrecords.php?illness=123

      The developer first applies the foreach loop in your question and then assumes "phew, I'm safe from SQL injection—nothing more to worry about!", so then does:

      $res = mysql_query("
        SELECT *
        FROM   health_records
        WHERE  user    = $_SESSION[uid]
           AND illness = $_GET[illness]
      ");
      while ($row = mysql_fetch_array($res)) print_r($row);
      

      Can you see the problem? Have a think about it.

      The problem

      Suppose someone requests the following URI:

      http://www.example.com/health/fetchmyrecords.php?illness=123+OR+1

      MySQL will then receive the following query:

      SELECT *
      FROM   health_records
      WHERE  user    = 987
         AND illness = 123 OR 1
      

      SQL successfully injected! In this particular example the injection is fatal, since MySQL will parse that WHERE clause as:

      WHERE (user = 987 AND illness = 123) OR 1
      

      …so it will actually return every health record of every user in the database!

      ^{Good luck affording the legal fees that will come out of that. It's a reasonable guess that your business will collapse, your personal finances will be ruined, your house will be repossessed, your wife will leave you, your kids won't speak to you and your cat will run away!}

      The explanation (for the injection, not your cat's disappearance) is that one can only escape strings with mysql_real_escape_string() (the clue is in the name). It can't be used to escape any other SQL token: not number literals; not identifiers; and certainly not SQL keywords or special characters. Those tokens must be secured in some other way.

      Thus arbitrarily escaping every received variable irrespective of its meaning is almost certainly incorrect. Furthermore, the results are only useful in SQL: should you want to use those variables for anything else, the data will be munged—so you really shouldn't store the results back in the $_GET and $_POST arrays anyway.

   2. It only escapes strings

      For the variables that are in fact strings, merely applying mysql_real_escape_string() won't necessarily stop SQL injection. You must also correctly quote the String Literals:

      A string is a sequence of bytes or characters, enclosed within either single quote (“'”) or double quote (“"”) characters.

      However, if the MySQL server is in the NO_BACKSLASH_ESCAPES SQL mode, then mysql_real_escape_string() does not safely escape strings for use within double quotes. So, to be safe, you must either:

      • use single-quoted literals; or

      • explicitly set some other SQL mode.

      Furthermore, before calling mysql_real_escape_string() you must have first told your MySQL client library with which character encoding the server will interpret such strings—usually by calling mysql_set_charset() (however, there are some encoding-related injections that even this won't defeat if you're using a really old version of the client library).

   See SQL injection that gets around mysql_real_escape_string() for more detail.

   Summing up 

   • use an up to date client library

   • call mysql_set_charset()

   • either disable NO_BACKSLASH_ESCAPES, or else use single-quoted literals

   • use mysql_real_escape_string() only on string literals

   • don't use the result for anything other than producing SQL

   • make other tokens safe in some other way

   
   

2. If there is a better more efficient way to make the connection more secure?

   Yes, there most certainly is—and has been for some years. In fact, so much so that mysql_real_escape_string() really ought to be considered an anti-pattern. It should never be used.

   Sadly, tutorial after tutorial continue to teach this ridiculously outdated and dangerous method of constructing SQL. Many of them have been around since before time itself, so it might be excusable that they were originally written badly (but by now they really ought to be updated, deleted or ignored); however for every new course or tutorial that recommends use of mysql_real_escape_string(), God kills a kitten. Won't somebody think of the kittens?!

   The "better way" is really two-fold:

   1. Stop using those PHP functions that are prefixed with mysql_. They were originally introduced in PHP v2.0 for MySQL v3.23, and no new features have been added since 2006. The manual has contained warnings against their use in new code since June 2011 and they have been officially deprecated in PHP v5.5.

      PHP ships with two perfectly good, modern alternatives: mysqli and PDO_MySQL—see MySQL: Choosing an API for help deciding which to use.

   2. Using one of those alternative APIs, you can parameterise your SQL statements so that literal values are transmitted to the server independently of the statement itself: thus the server will not even attempt to parse those literal values for SQL, irrespective of the values they contain. Injection simply cannot happen from a literal value (note that you will still need to protect other tokens, but that is a far less common requirement and beyond the scope of this answer).

   See How can I prevent SQL injection in PHP?

   
   

3. Will the connection still work and provide me with valid data from the database, if dbconnect.php were to be included in another file (for example; global.php), which in return was included in the main file where the actual use of database is?

   Yes. It is quite common in large projects for included files to include further files.