This post is going to talk about source code reviewing PHP and demonstrate how a relatively small chunk of code can cause you lots of problems.
The Code
In this article we are going to analyze the code displayed below. The code displayed below might seem innocent for some , but obviously is not. We are going to assume that is used by some web site to delete posts from the logged in users securely.
<?php
require_once 'common.php';
validatemySession();
mydatabaseConnect();
$username = $_SESSION['username'];// Insecure source
$username = stripslashes($username);// Improper filtering
$username = mysql_real_escape_string($username);//Flawed function
// Delete the post that matches the postId ensuring that it was created by this user
$queryDelete = "DELETE FROM posts WHERE PostId = " . (int) $_GET['postId']. " AND Username = '$username'";
if (mysql_query($queryDelete))// Bad validation coding {
header('Location: myPosts.php'); }
else {
echo "An error has occurred.";
}
?>
If you look carefully the code you will se that the code is vulnerable to the following issue: SQL Injection!!
Think this is not accurate , think better.
The SQL Injection
An adversary in order to exploit this vulnerability would not have to script custom tools, would only
have to have good knowledge of SQL injections methodologies and exposure to PHP coding.
Vulnerable Code:
Vulnerable Code:
1st Code Chunk
$username = $_SESSION['username'];// Insecure source
$username = stripslashes($username);// Improper filtering $username = mysql_real_escape_string($username);//Flawed function $queryDelete = "DELETE FROM posts WHERE PostId = " . (int) $_GET['postId']. " AND Username = '$username'"; if (mysql_query($queryDelete))
The mysql_real_escape_string function is based in the black list mentality. What it does is that escapes special characters in the un-‐escaped string, taking into account the current character set of the connection so that it is safe to place it in a mysql_query. More specifically mysql_real_escape_string calls MySQL's library function mysql_real_escape_string, which prepends backslashes to the following characters:
1. \x00
2. \n
3. \r
4. \
5. '
6. “
7. \x1a.
Due to this odd behavior characters such as the % and SQL keywords are not being affected, so queries that have the form of e.g. SELECT BENCHMARK(50,MD5(CHAR(118))) would be executed normally. A realistic scenario would be to use a query such as the one below:
Step1: SQL Payload that would cause the post to delete without the postId be known.
LIKE %’s’ Note1: At this point we assume that the attacker knows the format of the username e.g. its user plus a two-‐digit number.
Step2: SQL Payload mutated in order to bypass the filters.
LIKE CHAR(37, 8217, 115, 8217)
Note2: Further expanding on the attack if the Web App does not have a standard format for the
usernames then the adversary can brute-‐force the username first latter e.g. try out all English letters
e.g. LIKE %’a’, LIKE %’b’, LIKE %’c’ ... etc. and eventually execute the query with a valid first username
letter. Translating that to an obfuscated SQL Payload would be LIKE CHAR(39, 97, 37, 39), LIKE
CHAR(39, 98, 37, 39) etc.
Note3: The mysql_real_escape_string function is deprecated as of PHP 5.5.0, and will be removed in the future. Instead, the MySQLi or PDO_MySQL extension should be used. Alternatives to this function include: mysqli_real_escape_string and PDO::quote. The mysql_query() function sends a unique query (multiple queries are not supported) to the currently active database on the server that's associated with the specified link_identifier. In this specific code example the query returns true if a record is found (any record). This obscure behavior introduces the vulnerability combined of course with the above code. The function used in the mysql_query statement should return a record set only if the correct record set is returned and not just any match.
Note4: This extension is deprecated as of PHP 5.5.0.
Remedial Code:
Provide Server Side filters filter for remediating the vulnerability. Make use of strongly typed parameterized queries (using the bind_param).
Note3: The mysql_real_escape_string function is deprecated as of PHP 5.5.0, and will be removed in the future. Instead, the MySQLi or PDO_MySQL extension should be used. Alternatives to this function include: mysqli_real_escape_string and PDO::quote. The mysql_query() function sends a unique query (multiple queries are not supported) to the currently active database on the server that's associated with the specified link_identifier. In this specific code example the query returns true if a record is found (any record). This obscure behavior introduces the vulnerability combined of course with the above code. The function used in the mysql_query statement should return a record set only if the correct record set is returned and not just any match.
Note4: This extension is deprecated as of PHP 5.5.0.
Remedial Code:
Provide Server Side filters filter for remediating the vulnerability. Make use of strongly typed parameterized queries (using the bind_param).
// Using prepared Statements.
if ($stmt = $mysqli-‐>prepare("DELETE FROM posts WHERE PostId = ? AND Username = ? LIMIT 1"))
{
$stmt-‐>bind_param('s', $ username); // Bind "$ username" to parameter. $stmt-‐>execute(); // Execute the prepared statement.
...
}
Note: Other counter measures would include black list filtering of SQL keywords such as the LIKE and
CHAR keywords.
References:
References:
-
http://php.net/manual/en/function.mysql-‐real-‐escape-‐string.php
-
http://dev.mysql.com/doc/refman/5.0/en/pattern-‐matching.html
-
http://php.net/manual/en/function.mysql-‐query.php
-
http://www.php.net/manual/en/pdo.query.php
-
http://stackoverflow.com/questions/7069640/are-‐there-‐any-‐security-‐benefits-‐to-‐using-‐pdoquery-‐vs-‐mysql-‐query
