PHP Programming Guidelines (Part 2)

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:

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.

All In One Blog