Subtle security problems

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
User avatar
TehSausage
Forum Newbie
Posts: 1
Joined: Thu Oct 09, 2008 8:48 am
Location: Australia
Contact:

Subtle security problems

Post by TehSausage »

I was going to make some big long article, but I got bored. :P
Really, I just wanted an excuse to complain about Apache's *.php.* thing. =\
Does anyone want me to continue it?

> Subtle security problems

>>> Apache .php.* problem

If you think stopping a file from ending in .php is enough to stop it
from being executed, think again. (assuming this would also allow
other extensions such as .shtml, .pl etc. if they were enabled, but
they're ignored for now)

Code: Select all

 
/* Bad! */
 
if (substr($filename,-4) == '.php')
{
    exit("No scripts please.");
}
 
A file called test.php containing php code will be executed
A file called test.php.txt containing php code won't be executed
A file called test.php.jpg containing php code won't be executed
A file called test.php.fsdf containing php code will be executed
(unless you have set .fsdf as a known type =)

If a filename contains .php followed by a file extension unknown to
Apache (no mime type associated) - It will be executed if accessable
from a browser.
I'd call this a bug, but I'm sure it would have been picked up by the
Apache devs already if it was.

This problem affects the PHP module on both Linux and Windows Apache
servers.

The way I handle the problem is to block .*\.php.*$ on my server, but
this may be sufficient:

Code: Select all

 
/* "Okay"? */
 
if (strpos($filename,'.php') !== false) /* Block .php anywhere in filename */
{
    exit("No scripts please.");
}
 

>>> Poison Null (%00)

Once again, vunlerable on some webservers (*cough*Apache), but not others.

This is more well known than the above bug, but is just as dangerous.

Code: Select all

 
/* Seems harmless enough? Right? Wrong. */
/* Haha, and this is vulnerable to
   the above problem too */
/* Hell it's exactly the same example really... */
 
if (substr($_GET['filename'],-4) == '.php')
{
    exit("No scripts please.");
}
else
{
    file_put_contents($_GET['filename'],$some_user_inputted_data);
}
 
So anyway, the problem is that strings in PHP are not terminated by
null characters, but when passed through file_put_contents, and eventually
reaches the libc function "fopen" - the string has to be truncated
at where the null is.

Examples:
?filename=file.txt -- allowed
?filename=file.php -- blocked by script
?filename=file.php%00.txt -- allowed by script

The simplest solution is to simply truncate it before you check it

Code: Select all

 
/* Yeah this still still vulnerable to *.php.* on
   Apache, but it's just an example */
 
/* Terminate at null */
$filename = substr($_GET['filename'],0,(int)strpos($_GET['filename'],chr(0));
 
if (substr($filename,-4) == '.php')
{
    exit("No scripts please.");
}
else
{
    file_put_contents($filename,$some_user_inputted_data);
}
 


>>> Developing with magic_quotes enabled

If the config option magic_quotes_gpc is enabled, every value in $_GET,
$_POST and $_COOKIE will automatically be "escaped" (http://php.net/addslashes)
The other magic_quotes_* flags will also cause problems.

If your development machine has them enabled, and your production server
doesn't - It's not hard to accidently open up an SQL injection exploit.

Code: Select all

 
/* Safe on a server with magic_quotes_gpc */
/* Not safe at all otherwise */
 
mysql_query("SELECT * FROM users WHERE username = '{$_GET['username']}' AND password = '{$_GET['password']}'");
 
It can also cause problems such as \' appearing in user posts when the
situation is reversed:

Code: Select all

 
/* Escaping user input like a good boy */
 
$author = mysql_real_escape_string($_GET['author']);
$message = mysql_real_escape_string($_GET['message']);
 
mysql_query("INSERT INTO posts (author,message) VALUES ('$author','$message')");
 
Thank god it's non-existant in PHP 6+



> Easy fixes to common problems

>>> Restricting file access to one directory

Code: Select all

 
/* Bad! */
 
if (is_file('files/'.$_GET['filename']))
    readfile('files/'.$_GET['filename']);
 
Surely you'd never actually do that right... right?

The solution is simple, wrap the filename in basename()

Code: Select all

 
/* Better. */
 
if (is_file('files/'.basename($_GET['filename'])))
    readfile('files/'.basename($_GET['filename']));
 
There is a more complicated solution to allow sub-directory
access, but that won't be covered right now.
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Re: Subtle security problems

Post by alex.barylski »

I'd call this a bug, but I'm sure it would have been picked up by the Apache devs already if it was
I made the same mistake a few years back. I believe you can configure Apache to only execute PHP but I don't bother. Now I keep files outside docroot and use a proxy script to deliver the file -- problem solved.

I wouldn't consider it a bug it's actually handy. Consider implementing an image script with the GIF extension so it appears from a user point of view as a GIF file and not a PHP file.
If your development machine has them enabled, and your production server doesn't - It's not hard to accidently open up an SQL injection exploit.
No offense, but I would consider that an amatuer mistake and I think most would agree.

Besides escaping strings using addslashes() is not secure. You should use the RDBMS specific escaping functions.

http://shiflett.org/blog/2006/jan/addsl ... ape-string

Code: Select all

 
/* Better. */
 
if (is_file('files/'.basename($_GET['filename'])))
    readfile('files/'.basename($_GET['filename']));
 
Honestly. That is a horrible practice assuming this is actual code you have used. The fact that you have used a $_GET variable directly like this, tells me you are not doing three important things:

1. Filtering
2. Validating
3. Escaping

In that order on untrusted data. That is a bad practice and what leads to security holes and wonky bugs.
User avatar
Eran
DevNet Master
Posts: 3549
Joined: Fri Jan 18, 2008 12:36 am
Location: Israel, ME

Re: Subtle security problems

Post by Eran »

To add even more to Spectra's harsh but very correct words, you can stop execution at the server level by revoking execution privileges to the directories that store non-PHP files (especially upload and temporary folders). You absolutely do not want to mix application files (i.e PHP files) and files accessible by users.

And of course, everything Spectra said :P

I'm sure Mordred has a thing or two to add...
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: Subtle security problems

Post by Mordred »

The unobvious behaviour of Apache with multiple extensions is a security must-know, so in that regard I think it's a useful post.

It is also wrong ;) Here's why: The gist of TehSausage's post seems to address situations where we work with user-uploaded files. In this situation, my personal oppinion is that "filtering" filenames is a wrong thing to do. First, you may get it wrong. Second, it will protect against one attack only. I would rather have a security-in-depth solution, preventing multiple known (and hopefully some unknown) attacks.

The best strategy would be to accept any file, but store it in a secret location with a random filename, and serve it through a proxy script (if it has to be served directly of course). Anything less opens possibilities for the attacker in the presence of other security holes - LFI being the most common.
Post Reply