4. SQL-injection
The term SQL-injection is used to describe the injection of commands into an
existing SQL query. The Structured Query Language (SQL) is a textual language
used to interact with database servers like MySQL, MS SQL and Oracle.
Why not start out with an example?
$iThreadId = $_POST['iThreadId'];
// Build SQL query
$sSql = 'SELECT sTitle FROM threads
WHERE iThreadId = ' . $iThreadId;
To see what's wrong with to code above, let's take a look at the following
HTML code:
<form
method="post" action="insecure.php">
<input
type="text" name="iThreadId" value="4; DROP TABLE users" />
<input
type="submit" value="Don't click here" />
</form>
If we submit the above form to our insecure page, the string sent to the
database server would look like the following, which is not very pleasant:
SELECT sTitle FROM
threads WHERE iThreadId = 4; DROP TABLE users
There are several ways you can append SQL commands like this, some dependent
of the database server.
To take this further, this code is common in PHP applications:
$sSql = 'SELECT
iUserId FROM users WHERE sUsername = \'' . $_POST['sUsername'] . '\' AND sPassword = \'' . $_POST['sPassword'] . '\'';
// Or (depending on writing style):
$sUsername = $_POST['sUsername'];
$sPassword = $_POST['sPassword'];
$sSql = "SELECT iUserId FROM users
WHERE sUsername = '$sUsername' AND sPassword = '$sPassword'";
We can easily skip the password section here by entering
"theusername'--" as the username or "' OR '' = '" as the
password (without the double-quotes), resulting in:
# Note: -- is a line comment in MS SQL so
everything after it will be skipped
SELECT iUserId FROM users WHERE sUsername = 'theusername'--' AND sPassword = ''
# Or:
SELECT iUserId FROM users WHERE sUsername = 'theusername' AND sPassword = '' OR
'' = ''
Here is where validation comes into play, in the first example above we must
check that $iThreadId really is a number before we append it to the SQL-query.
if ( !
is_numeric($iThreadId) )
{
// Not a number, output error
message and exit.
...
}
The second example is a bit trickier since PHP has built in functionality to
prevent this, if it is set. This directive is called magic_quotes_gpc, which
like register_globals generally is a bad thing. In my opinion that is, and I
will explain why.
To have characters like ' in a string we have to escape them, this is done
differently depending on the database server:
# MySQL:
SELECT iUserId FROM users WHERE sUsername = 'theusername\'--' AND
sPassword = ''
# MS SQL Server:
SELECT iUserId FROM users WHERE sUsername = 'theusername''--' AND sPassword = ''
Now what magic_quotes_gpc does, if set, is to escape all input from GET,
POST and COOKIE (gpc). This is done as in the first example above, that is with
a backslash. So if you enter "theusername'--" into a form and submit
it, $_POST['sUsername'] will contain "theusername\'--", which is
perfectly safe to insert into the SQL-query, as long as the database server
supports it (MS SQL Server doesn't). This is the first problem the second is
that you need to strip the slashes if you're not using it to build a SQL-query.
A general rule here is that we want our code to work regardless if
magic_quotes_gpc is set or not. The following code will show a solution to both
examples:
// Strip backslashes from GET, POST and
COOKIE if magic_quotes_gpc is on
if ( get_magic_quotes_gpc() )
{
// GET
if ( is_array($_GET) )
{
// Loop
through GET array
foreach( $_GET as $key =>
$value )
{
$_GET[$key] =
stripslashes($value);
}
}
// POST
if ( is_array($_POST) )
{
// Loop
through POST array
foreach( $_POST as $key =>
$value )
{
$_POST[$key] =
stripslashes($value);
}
}
// COOKIE
if ( is_array($_COOKIE) )
{
// Loop
through COOKIE array
foreach( $_COOKIE as $key =>
$value )
{
$_COOKIE[$key] =
stripslashes($value);
}
}
}
function sqlString
($sText)
{
if ( is_null($sText) )
{
return
'NULL';
}
else
{
//
MySQL
if
( ...
)
return '\'' .
mysql_real_escape_string($sText) .
'\'';
// MS
SQL
else
return '\'' .
str_replace('\'',
'\'\'',
$sText) .
'\'';
}
}
function sqlNumber
($fNumber)
{
if (
!
is_numeric($fNumber) )
return
'NULL';
else
return
str_replace(',',
'.',
(string
) $fNumber);
}
// First example
$iThreadId =
$_POST['iThreadId'];
$sSql =
'SELECT sTitle FROM threads
WHERE iThreadId = ' . sqlNumber
($iThreadId);
// Second example
$sUsername =
$_POST['sUsername'];
$sPassword =
$_POST['sPassword'];
$sSql =
'SELECT iUserId FROM users WHERE
sUsername = ' . sqlString
($sUsername) .
' AND
sPassword = ' . sqlString
($sPassword);
Now we have a straightforward way of building safe SQL queries. Preferably
we put the first statements and functions in an include for easy reuse.
The PHP function mysql_real_escape_string escapes the following characters:
NULL, \x00, \n, \r, \, ', " and \x1a. So it is highly recommended when
working with MySQL.
Another function call worth mentioning here is set_magic_quotes_runtime(0)
which turns of magic_quotes_runtime. This directive if set escapes data
returned from external resources like databases and files. I would recommend
adding it to the code above.
Now as you probably can imagine a malicious user can do a lot more than what
I've shown you here, that is if we leave our scripts vulnerable to injection. I
have seen examples of complete databases being extracted from vulnerabilities
like the ones described above.
5. Target functions
The following is a list of functions to be extra careful with. If unfiltered
input get to one of these functions exploitation is often possible.
5.1. Execution of PHP code
include() and require() - Includes and evaluates a file as PHP code. eval()
- Evaluates a string as PHP code. preg_replace() - The /e modifier makes this
function treat the replacement parameter as PHP code.
5.2. Command injection
exec(), passthru(), system(), popen() and the backtick operator (``) - Executes
its input as a shell command.
When passing user input to these functions, we need to prevent malicious
users from tricking us into executing arbitrary commands. PHP has two functions
which should be used for this purpose, they are escapeshellarg() and
escapeshellcmd().
6. Configuration settings
6.1. register_globals
If set PHP will create global variables from all user input coming from get,
post and cookie.
If you have the opportunity to turn off this directive you should definitely
do so. Unfortunately there is so much code out there that uses it so you are
lucky if you can get away with it.
Recommended: off
6.2. safe_mode
The PHP safe mode includes a set of restrictions for PHP scripts and can
really increase the security in a shared server environment. To name a few of
these restrictions: A script can only access/modify files and folders which has
the same owner as the script itself. Some functions/operators are completely
disabled or restricted, like the backtick operator.
6.3. disable_functions
This directive can be used to disable functions of our choosing.
6.4. open_basedir
Restricts PHP so that all file operations are limited to the directory set
here and its subdirectories.
6.5. allow_url_fopen
With this option set PHP can operate on remote files with functions like
include and fopen.
Recommended: off
6.6. error_reporting
In the development process we want to write as clean code as possible and
thus we want PHP to throw all warnings etc at us.
Recommended for development: E_ALL
6.7. log_errors
Logs all errors to a location specified in php.ini.
Recommended: on
6.8. display_errors
With this directive set, all errors that occur during the execution of
scripts, with respect to error_reporting, will be sent to the browser. This is
desired in a development environment but not on a production server, since it
could expose sensitive information about our code, database or web server.
Recommended: off (production), on (development)
6.9. magic_quotes_gpc
Escapes all input coming in from post, get and cookie. This is something we
should handle on our own.
This also applies to magic_quotes_runtime.
Recommended: off
6.10. post_max_size, upload_max_filesize and
memory_limit
These directives should be set at a reasonable level to reduce the risk of
resource starvation attacks.
7. Recommended practices
7.1. Double versus Single quotes
// Double quotes:
echo 'Echoing a
line with linefeed.',
"\n";
// Single quotes:
$sSql =
'SELECT iUserId FROM users WHERE
sName = ' . sqlString
($sName);
echo '<td
width="100"></td>';
As a general rule use double quotes with tabs, linefeed etc. For example
"\t", "\n". In all other cases use single quotes.
7.2. Do not rely on short_open_tag
If short_open_tag is set it allows the short form of PHP's open tag to be
used, that is <? ?> instead of <?php ?>. Like register_globals you
should never assume that this is set.
7.3. String concatenation
$sOutput = 'Hello '
. $sName;
// Not: $sOutput = "Hello $sName";
This increases readability and is less error prone.
7.4. Comment your code
Always try to comment your code, regardless of how simple it may seem to you
and remember to use English.
7.5. Complex code should always be avoided
If you find that you have trouble understanding code you've written then try
to picture other people understanding it. Comments help but doesn't always do
it here so rewrite!
8. Naming Conventions
8.1. Variable names should be in mixed case
starting with lower case prefix
$sName, $iSizeOfBorder
// string, integer
This makes it very easy to look at a variable and directly see what it
contains.
Here is a list of common prefixes: a Array b bool d double f float i int l
long s string g_ global (followed by normal prefix)
8.2. Boolean variables should use the prefix
b followed by is, has, can or should
$bIsActive, $bHasOwner
This also applies to functions that return boolean, but without the b
prefix, for example:
$user->hasEmail();
8.3. Negated boolean variable names must be
avoided
var $bIsActive;
// Not: $bIsNotActive
var $bHasId;
// Not: $bHasNoId
It's not directly obvious what that following code does:
if ( !$bIsNotActive )
{
...
}
8.4. Object variables should be all
lowercase or use the prefix o or obj
$session, $page
// Preferred
$oSession
$objPage
All lowercase is the preferred here but the important thing is to be
consistent.
8.5. Constants must be all uppercase using
underscore to separate words
$SESSION_TIMEOUT, $BACKGROUND_COLOR,
$PATH
However, in general the use of such constants should be minimized and if
needed replace them with functions.
8.6. Function names should start with a verb
and be written in mixed case starting with lower case
validateUser(), fetchArray()
8.7. Abbreviations must not be uppercase
when used in a name
$sHtmlOutput
// Not: $sHTMLOutput
getPhpInfo()
// Not: getPHPInfo()
Using all uppercase for the base name will give conflicts with the naming
conventions given above.
8.8. SQL keywords should be all uppercase
SELECT TOP 10
sUsername, sName FROM users
This makes it much easier to understand and get an overview of a SQL query.
8.9. Private class variables should have
underscore suffix
class MyClass
{
var $sName_;
}
This is a way of separating public and private variables. However this is
not as important as in other languages since in PHP you use the $this->
pointer to access private variables.
8.10. All names should be written in English
$iRowId // Not:
$iRadId (Swedish)
English is the preferred language for international development. This also
applies to comments.
8.11. Variables with a large scope should
have long names, variables with a small scope can have short names
Temporary variables are best kept short. Someone reading such variables
should be able to assume that its value is not used outside a few lines of
code.
Common temporary variables are $i, $j, $k, $m and $n. Since these variables
should have small scope prefixes are a bit of overkill.
8.12. The name of the object is implicit,
and should be avoided in a method name
$session->getId() // Not: $session->getSessionId()
The latter might seem natural when writing the class declaration, but is
implicit in use, as shown in the example above.
8.13. The terms get/set must be used where an attribute is accessed
directly
$user->getName()
$user->setName($sName)
This is already common practice in languages like C++ and Java.
8.14. Abbreviations should be avoided
$session->initialize();
// Not: $session->init();
$thread->computePosts();
// Not: $thread->compPosts();
There is an exception to this rule and that is names that are better known
for their shorter form, like Html, Cpu etc.
9. Syntax
9.1. Function and class declarations
function doSomething()
{
...
}
// Not:
function doSomething() {
...
}
The same applies to class declaration.
9.2. Statements and curly brackets
if ( $bIsActive )
{
...
}
// Not:
if ( $bIsActive
) {
...
}
The first bracket should always be on the line after the statement, not on
the same line. The code is much easier to follow this way. This also applies to
for, switch, while etc.
9.3. Statements and spaces
if ( $sName == 'John' )
{
...
}
// Not:
if($sName=='John')
{
...
}
10. Summary
If I were to summarize this chapter in one word it would be validate, I can
not stress this enough. Validate, validate and validate. Do not trust input
from any source unless you can be 100% certain that it has not been tampered
with. This applies to variables in global scope as well as input from GET, POST
and COOKIE. Even data in a database can not be trusted if it sometime came from
user input.
