views:

302

answers:

4

I am trying to create a function that will block access to some of my low level directories. For example, while building my site I don't want to allow uploads to be uploaded any lower than /var/www/html/site/uploads/ should I make a mistake while coding. This will also help to prevent a directory delete typo while removing cache files or whatever.

This is easily done with realpath() and strcasecmp().

The problem is that I can't use realpath() to generate an absolute path because any calls to this function with directories that don't exist will return FALSE. Below is my best attempt at looking at paths to validate them.

function is_sub_dir($path = NULL, $parent_folder = NULL) {

    //Convert both to real paths
    //Fails if they both don't exist
    //$path = realpath($path);
    //$parent_folder = realpath($parent_folder);

    //Neither path is valid
    if( !$path OR !$parent_folder ) {
     return FALSE;
    }

    //Standarize the paths
    $path = str_replace('\\', '/', $path);
    $parent_folder = str_replace('\\', '/', $parent_folder);

    //Any evil parent directory requests?
    if(strpos($path, '/..') !== FALSE) {
     return FALSE;
    }

    //If the path is greater
    if( strcasecmp($path, $parent_folder) > 0 ) {
     return $path;
    }

    return FALSE;
}

//BAD FOLDER!!!
var_dump(is_sub_dir('/var/www/html/site/uploads/../', '/var/www/html/site/uploads/'));

Does anyone know how to properly put file path blocks in place to guard against low-level folder access?

:UPDATED:

I want to further explain that this checking method will be used on multiple severs as well as in the creation of directories that are higher than a given directory.

For example, in my uploads directory I wish to allow administrators to create new sub directories such as ...uploads/sub/. By figuring out a reliable way to insure that the directory given is in fact higher than the parent directory - I can feel safer allowing my admins to work with the file system in the uploads folder.

So since I might need to verify that uploads/sub is higher than uploads/ before I create it I can't use realpath() because uploads/sub doesn't exist again.

As for the actual location of the uploads folder that is figured on the fly by PHP.

define('UPLOAD_PATH', realpath(dirname(__FILE__)));

:UPDATE 2:

I have an idea, what if I were to use realpath to compare the the whole path minus the last directory segment. Then even if that last directory segment still needed to be created - the rest of the path could be forced to match the minimum parent directory?

A: 

Is it as easy as this?

const UPLOAD_DIR = '/var/www/site/uploads/';

Every time you need to use it you just use the constant since it sounds like there is only one place you can upload. That way you can't screw up as easily.

if ($dir != UPLOAD_DIR) {
   // No access; Error
}

Sometimes to protect you from yourself you just have to be vigilant. Just make sure to call your is_sub_dir() before all file access.

EDIT:

Now that the question is more clear I see my answer makes no sense. =) My only other advice is to reiterate what others have said: sanitize, sanitize, sanitize.

Chris Kloberdanz
Actually, the problem isn't checking that a directory is the same as another - it is checking that a directory is the child of another.
Xeoncross
I guess I just figured since you were only allowing uploads in one directory that comparing the directory you are trying to write to with the expected one would be sufficient. Any subdir wouldn't be allowed as well any other dir.
Chris Kloberdanz
What happens if somebody wants to write a file named "../../../../bin/sh"?
jprete
His question has been updated to better explain himself so now my comment should be disregarded.
Chris Kloberdanz
+4  A: 

Not an answer to your immediate question but you should never use blacklists in a real security environment.

That's because you may change the directory layouts but forget to update the blacklist, leaving your security impotent.

With a whitelist, you list the places where they're allowed to upload to. If you then change the layout and forget to change the whitelist, your security has actually increased, not decreased. And the howls of protest from your users will alert you to your forgetfulness.

In terms of disallowing access to lower-level directories, I would think it's a simple matter of getting the real path (with all those ./ and ../ and \\ converted to a normalized form) and then, if it starts with /var/www/html/site/uploads/, fail if there's any more / characters after that.

I've done this normalization before and it basically consists of (from memory):

  1. Replace all \\ with / (so it uses UNIX-isms).
  2. If it doesn't start with /, add your base directory at the front (so it's always an absolute path).
  3. Replace all /./ with / (gets rid of useless "stay in current directory" moves).
  4. Replace all /X/../ with / where X is any non-/ character (gets rid of down-up directory moves).
  5. Then make sure it's a valid location.

What's left is generally safe to use, though there may be edge cases depending on whether there are more directory-move commands available (I've seen ... as the quivalent to ../..). Your mileage may vary.

paxdiablo
Thanks I updated my question after reading this. The problem is that some of the folders requested might not be valid locations - *yet*. Also, I was unaware of the "..." shortcut.
Xeoncross
When I say "valid", I don't mean that it exists, just that it isn't a bad location (like /var/data/../../etc/passwd). The method is based entirely on string processing, not anything to do with what's on the filesystem.
paxdiablo
+3  A: 

The best way I know of is to simply make everything but the upload directory unwritable by the user the web server runs as. For instance, on Debian, Apache runs as the user www-data. So, I make sure all the directories that Apache might serve out as world readable/executable, owned by some user other than www-data, and only writable by that user (or some admin group). Then, if a directory needs to be writable by the web server, I make it writable by www-data, either via group or owner.

If it's possible, it's also a good idea to move writable directories outside of the main server tree. If I control the server, I create a directory under /var/lib explicitly for this purpose. If I don't control the server, I put it next to the served directories. For instance, using your example of /var/www/site, I'd add another level, say /var/www/site/html (for direct-served HTML and top level scripts), /var/www/site/scripts (for include()ed PHP scripts) and /var/www/site/data (for uploaded data). Of course, if you need to serve the uploaded data, you either need to write a PHP wrapper to serve them out or put them under /var/www/site/html/uploads, similar to how you're doing things now.

Michael Johnson
Yes, this is the most secure method. However, on some of the servers this will be deployed on, administrators will be able to create new directories in the uploads folder - hence the problem of checking for a valid path.
Xeoncross
It should still be possible to use this method. If you set the group sticky bit, the group remains the same for subdirectories. Then, if your administrators are members of that group they can create subdirectories and they are still viable.
Michael Johnson
A: 

The following code works even if the last directory in the path doesn't exist yet. Any foul play or additional missing directories returns false.

The limit to this function is that it only works with paths that (mostly) already exist and directory names using standard English chars (/.hts/, /files.90.3r3/, /my_photos/) etc..)

function is_sub_dir($path = NULL, $parent_folder = SITE_PATH) {

    //Get directory path minus last folder
    $dir = dirname($path);
    $folder = substr($path, strlen($dir));

    //Check the the base dir is valid
    $dir = realpath($dir);

    //Only allow valid filename characters
    $folder = preg_replace('/[^a-z0-9\.\-_]/i', '', $folder);

    //If this is a bad path or a bad end folder name
    if( !$dir OR !$folder OR $folder === '.') {
     return FALSE;
    }

    //Rebuild path
    $path = $dir. DS. $folder;

    //If this path is higher than the parent folder
    if( strcasecmp($path, $parent_folder) > 0 ) {
     return $path;
    }

    return FALSE;
}
Xeoncross
Note: *DS* = DIRECTORY_SEPARATOR
Xeoncross
You should change !$dir to $dir === false (same with !$folder) to properly check for false. Also, strcasecmp() is the wrong method to use, I believe. strpos($path, $parent_folder) > 0 is the correct comparison I think. The call in the posted code checks if $parent_folder is lexically greater than $path - so strcasecmp("z", "a") would pass. Obviously not what you're looking for. Finally, I wouldn't do a case-insensitive comparison here, unless you're running exclusively on Windows. Otherwise gooddir/path would compare equal to GOODDIR/path, which under Linux would be two different directories.
Michael Johnson