Forum Moderators: coopster

Message Too Old, No Replies

Writing secure PHP scripts - my thoughts

finding and fixing security problems

         

Aleister

12:29 am on Aug 16, 2005 (gmt 0)

10+ Year Member



I have a decent amount of experience with php doing little projects, but lately I have started using it more and more. After a lot of reading, I have started to implement better security measures.

I know there is a lot of information out there on this, but these are a few points I would like to make from what I have learned. Maybe this will be useful to someone, or maybe someone else will suggest something I have not thought of :)

Here are a few functions that I have been using.

fix_for_page - I use this to take form data and make it safe to display on the page. This is after my standard validation (size checks, what kind of data do I need, etc..). It seems to work nicely.


function fix_for_page($value){
$value = htmlspecialchars(trim($value));
if (get_magic_quotes_gpc())
$value = stripslashes($value);
return $value;
}

fix_for_mysql - I use this when I am taking user submitted data, and entering it into a query. Once again, I do the data validation first to make sure I am getting the kind of info I want.


function fix_for_mysql($value){
if (get_magic_quotes_gpc())
$value = stripslashes($value);
$value = "'" . mysql_real_escape_string($value) . "'";
return $value;
}

It is important to note that these functions can not (properly) be both used on the same variable, since they each have their separate uses. fix_for_page escapes characters important for html output, while for_for_mysql misses some characters (that would make it bad to display in the browser), but that is ok, since it is escaping properly for a query usage.

The function also adds single quotes around the query, because I use it like this:


$q = sprintf("select password from users where username = %s", $username);
$result = mysql_query($q, $dbc);

This way, it should be safe from any strange input because it is securing it completely within the validation and fix_for_mysql function and not relying on any 'fixing up' from the query string.

I finally realized after a while that is is very important to use htmlspecialchars and mysql_real_escape_string when they need to be used, and not just using one for both needs. This ensures that everything gets escaped properly, depending on what you want to do with it.

One other note about the "form <-> page <-> query" conversions, I find that if I am using a textarea, I use the standard fix_for_mysql to enter it into the database, but when I want to display the data on the page (after taking it back from the database), this method works well:


nl2br(htmlspecialchars($dbarray['intro'])

This is because if I use my standard fix_for_page function on multi-line entities, the line breaks will not be handled properly, especially since it will contain newlines instead of breaks once that data is sent to mysql.

Aside from _always_ protecting user input, another important consideration is undeclared variables. Here is a scenario which many have written, but perhaps not all have thought about.

Say you have a /template/header.php which contains the line:


<title><?php echo $page_title;?></title>

And each of your files that call the header start like this:


<?
include 'includes/db_connect_stuff.php';
$page_title = "Index";
include 'template/header.php';
?>

Everything seems safe, since you are declaring $page_title before calling the header. But what if someone simply calls the header directly like this:


www.example.com/template/header.php?page_title=blah..

Then you have a security risk. This is just a basic example of course. It is easy to fix, just change your header.php to this:


<title><?php echo fix_for_page($page_title);?></title>

Once you start looking deeper into your code, things like this will be obvious, although some more than others, at least at first.

Just as you can never trust user input, never trust the contents of any variable unless you know exactly how your code will handle any situation.

Now lets look at form mailers. The same applies. Always validate, always escape data. Many people think that since the "TO" field is hard-coded in their script, there is no way it can be used to send email to others (by a bot, spamming people), but this is not true. All it takes is an extra set of headers injected into (pretty much any) form field. Simple character escaping and validation will prevent this.

That is all for now. Please feel free to comment, so that I know I am on the right track, or about to crash into a wall ;)

(Sometime I will re-write this so that it looks presentable heh)

jatar_k

12:59 am on Aug 16, 2005 (gmt 0)

WebmasterWorld Administrator 10+ Year Member



a large majority of my thoughts were written here
PHP Security [webmasterworld.com]
PHP Peer Code Review [webmasterworld.com]

those functions look fine, they wrap the standard escaping functions with a check for magic_quotes_gpc. Good advice for people who write on shared hosts or distribute scripts.

Your example for GET vars overwriting values is ok but with register_globals off it won't be a problem.

the thing that comes to mind over and over is a very important point about php in any environment

Know Your Environment

know your configuration - run phpinfo() find out how everything is configured. is register_globals on? is fopen url enabled? what extensions and extras are enabled, which ones aren't and why?

know your server - if you're on a shared host find out about the server your site is running on. Find out if your host has compromised your security before you even get a chance to start.

know your scripts - if you use snippets or scripts that were written by other people then figure out exactly what they are doing and whether you might have problems in the past. Customize them to your needs, hack them to be more secure, send the patches to the authors if you can or just document them in case you need to upgrade the script.

jd01

9:30 am on Aug 16, 2005 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Some of my thoughts:

1. Register Globals & error reporting off

2. Rewrite URL's to static .html

3. Deny browser or link click requests to .php files.

RewriteCond %{THE_REQUEST} ^[A-Z]{3,9}\ /[^.]+\.php.*\ HTTP/
RewriteRule \.php - [F]

4. Strip the ? and everything after from original (browser or link click) requests so all Query_Strings that make it to the php page have qualified for the rewrite regular expression before they ever make it to the php page.

RewriteCond %{THE_REQUEST} \?.+\ HTTP/ [NC]
RewriteRule (.*)$ /$1? [R=301,L]

5. Check all variables one more time on the way in.

Justin

dmorison

12:02 pm on Aug 16, 2005 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Just remember to delete that phpinfo.php script you created from your production server, unlike, hmmmmm....?

[google.co.uk...]

...15,600 other people!

dreamcatcher

1:07 pm on Aug 16, 2005 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Amazing. LOL :)

stidj

4:23 pm on Aug 16, 2005 (gmt 0)

10+ Year Member



Very helpful, thanks a lot guys.

Another thing I recommend is for dynamic PHP...automatically iterate through the POST array when working with forms and strip all bad stuff out of them like trim() and htmlspecialchars so you are more likely to have something safe/sanitized before it even hits the rest of your code

Aleister

10:38 pm on Aug 16, 2005 (gmt 0)

10+ Year Member



I noticed many good points which I had forgot while typing that! :) Thanks for the input!

Now it is just a matter of trying to 'break' my script to test :)