Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Securing paths in PHP

Tags:

security

php

I'm writing some PHP which takes some paths to different content directories, and uses these to include various parts of pages later. I'm trying to ensure that the paths are as they seem, and none of them break the rules of the application. Which are,

  1. PRIVATEDIR (defined relative to DOCUMENT_ROOT) must be above DOCUMENT_ROOT.
  2. CONTENTDIR (defined relative to PRIVATEDIR) must lie below PRIVATEDIR and must not go back in to DOCUMENT_ROOT.
  3. The remaining *DIRS's (defined relative to CONTENTDIR) must lie below CONTENTDIR

I'm setting some defaults in a singleton controller class, and then the user passes in an array of the paths they want to override to this classes constructor. I then want to sanity check them to ensure they abide by the above rules. Heres how I've started to go about it...

EDIT: Please note my use of error_reporting in the code below, and then don't do it yourself! I misunderstood how that command works. If you're wondering why, see stealthyninja's and Col. Shrapnel's remarks in the comments (and thanks to them for pointing this out to me).

private $opts = array( // defaults
   'PRIVATEDIR'   => '..',        // relative to document root
   'CONTENTDIR'   => 'content',   // relative to private dir
   ...
   ...
);

private function __construct($options) { //$options is the user defined options
    error_reporting(0);
    if(is_array($options)) {
        $this->opts = array_merge($this->opts, $options);
    }

    if($this->opts['STATUS']==='debug') {
        error_reporting(E_ALL | E_NOTICE | E_STRICT);
    }

    $this->opts['PUBLICDIR']  = realpath($_SERVER['DOCUMENT_ROOT'])
                                        .DIRECTORY_SEPARATOR;
    $this->opts['PRIVATEDIR'] = realpath($this->opts['PUBLICDIR']
                                        .$this->opts['PRIVATEDIR'])
                                        .DIRECTORY_SEPARATOR;
    $this->opts['CONTENTDIR'] = realpath($this->opts['PRIVATEDIR']
                                        .$this->opts['CONTENTDIR'])
                                        .DIRECTORY_SEPARATOR;
    $this->opts['CACHEDIR']   = realpath($this->opts['CONTENTDIR']
                                        .$this->opts['CACHEDIR'])
                                        .DIRECTORY_SEPARATOR;
    $this->opts['ERRORDIR']   = realpath($this->opts['CONTENTDIR']
                                        .$this->opts['ERRORDIR'])
                                        .DIRECTORY_SEPARATOR;
    $this->opts['TEMPLATEDIR' = realpath($this->opts['CONTENTDIR']
                                        .$this->opts['TEMPLATEDIR'])
                                        .DIRECTORY_SEPARATOR;

   ...
   ...
   ...


    // then here I have to check that PRIVATEDIR is above PUBLICDIR
    // and that all the rest remain within private dir and don't drop 
    // down into (or below) PUBLICDIR again. And die with an error if
    // they don't conform.
}

The thing is this seems like a lot of work to do, especially as it must be run, every time a page is accessed, before I can do anything else (e.g check for a cached version of the page I'm serving), when the paths are essentially static.

Part of me is thinking that the maintainer (currently me) of the site should be aware of what paths they are supplying and should check themselves that they conform to the rules. But the (I think) more sensible side of me is saying, no, I should be equally responsible for checking that the paths conform, since accidents do happen (especially as the site grows/changes/gets a new maintainer...) and if they can be caught, in a security-critical environment, there is no excuse not to catch them.

So, I'm pretty much decided that I DO want to check these paths, the question is how good is my method? It feels like a suboptimal solution. How would you improve it or what alternatives would you suggest? Is there anyway of getting round the problem of having to do it every time a page loads, and somehow just do it when the configuration changes? (That would be ideal, but I don't imagine trivial).

Thanks.

like image 940
tjm Avatar asked Nov 06 '22 05:11

tjm


1 Answers

How about using strpos to find a substring? It's not the most elegant solution, but if you just want some basic protection for someone who already is given a lot of power on the site. Here is how I would do it:

// PRIVATEDIR already includes PUBLICDIR at this point
if(strpos($this->opts['PRIVATEDIR'],$this->opts['PUBLICDIR']) !== 0) {
die('ERROR: '.$this->opts['PRIVATEDIR'].' must be located in .'$this->opts['PUBLICDIR']);
}

Note the triple not equals, that makes sure that PUBLICDIR occurs at position 0 of the PRIVATEDIR string and not it simply being not found or somewhere else in the string. Also you should check that they aren't the same (PUBLICDIR != PRIVATEDIR) since you could set it to "./test/../" and that would return you to just ".".

like image 183
Michael Petrov Avatar answered Nov 09 '22 16:11

Michael Petrov