Note: This post is targeted at MOD authors and contains many technical details.
Introduction
Amongst the great security features that phpBB 3.0 provides is the function used for processing user input, request_var. This function was designed to make it easy to securely retrieve user inputted data. It is one of the most important security functions in a system that retrieves external data as it can (with caveats that will be elaborated upon) single-handedly stop XSS and SQL injection attacks dead in their tracks
The reason we have created this blog post is to give more information to modification developers on how to properly explain how this works and why you should use it.
SQL Injections
SQL injections are one of the most dangerous vulnerabilities a web application can have, yet are one of the easiest vulnerabilities to prevent. These attacks happen when data is inserted into the database without properly sanitizing. Anything from the $_GET, $_POST, $_COOKIE, or $_SERVER arrays should be considered user input and require sanitizing.
To prevent SQL injections, proper use of request_var’s data typing is required.
Data typing
In order to prevent SQL injections on non-strings it is important to set the data type on the input. It is absolutely crucial to make sure the type is what you expect it to be.
In request_var this is done by casting the second argument, the default value, to the type you need.
Example usage
Here is an example of how to use request_var. It obtains user input by using request_var and runs an SQL query based on it. Note the 0 in the request_var call, it makes sure that $user_id is a number.
- Code: Select all
$user_id = request_var('user_id', 0);
$sql = 'SELECT username
FROM ' . USERS_TABLE . '
WHERE user_id = ' . $user_id;
$result = $db->sql_query($sql);
$row = $db->sql_fetchrow($result);
$db->sql_freeresult($result);
echo $row['username'];
Where things can go wrong
Here is an example of a mistake which can have very bad consequences. It is based on the previous example. Instead of defaulting to 0 we want to default to the current user’s user_id.
- Code: Select all
$user_id = request_var('user_id', $user->data['user_id']);
$sql = 'SELECT username
FROM ' . USERS_TABLE . '
WHERE user_id = ' . $user_id;
$result = $db->sql_query($sql);
$row = $db->sql_fetchrow($result);
$db->sql_freeresult($result);
echo $row['username'];
As you can see, only the first line has changed.
The vulnerability
Did you spot it? $user->data[‘user_id’] is a value that is taken directly from the database and all values from the database are strings. This means that $user_id will not be cast to an integer but instead to a string. The consequence is that an attacker can alter the SQL query to one that retrieves or changes specific data in your database for their own use.
If, for example, an attacker wanted to get some email addresses, he could simply set the user_id GET parameter to:
- Code: Select all
0 UNION SELECT user_email as username FROM phpbb_users WHERE user_id = 2
This would result in a final SQL query like this:
- Code: Select all
SELECT username FROM phpbb_users WHERE user_id = 0 UNION SELECT user_email as username FROM phpbb_users WHERE user_id = 2
That would give him the email address of the user with user_id 2. It should be pretty clear that there is a lot of bad stuff that can be done using this method.
How to do it properly
It is extremely easy to prevent this from happening, the only required change is casting the second argument of request_var to an integer. In code this would look as follows.
- Code: Select all
$user_id = request_var('user_id', (int) $user->data['user_id']);
One more note for strings
Although request_var can protect you for most types when properly used, there is still one more requirement if you are requesting a string. That is to escape the string using the correct escaping method for your database, which is all handled correctly using the sql_escape function in the database object.
- Code: Select all
$foo = request_var('foo', 'bar');
$sql = 'SELECT username
FROM ' . USERS_TABLE . '
WHERE foo = ' . $db->sql_escape($foo);
That’s it, folks!
request_var is a great tool but it needs to be used correctly, which includes casting the default value to the correct type and escaping any strings before using them in an SQL query.
Resources
It is one of the most important security functions in a system that retrieves external data as it can (with caveats that will be elaborated upon) single-handedly stop XSS and SQL injection attacks dead in their tracks.
Good advice.
To add: don’t quote numbers! Just don’t do it anywhere, unless you specifically want to output it as a literal.