4
\$\begingroup\$

I wanted to load every page under index.php with a simple index.php?do=login

For example when the link index.php?do=login or /login/ is opened, the file login.php is loaded (from outside public_html), and so everything is done under index.php instead of having a separate php file in public_html for each action!

This is what I've tried already, am I doing it okay? (I'm not sure if what I'm doing has any security vulnerabilities, so please kindly advise)

index.php

<?php

// application path (outside public_html)
define('FILESPATH', APP.'../application/actions/');

// Default file
$file_name = 'home_page';

// e.g. index.php?do=login or /login/
if(isset($_GET['do'])){
    $file_name = rtrim($_GET['do'],'/');
}

// Set file path
$fpath = FILESPATH."{$file_name}.php";

// Make sure user input has valid characters a-z 0-9 _
if(preg_match('/^[A-Za-z0-9_]+$/', $file_name) !== 1 OR ! file_exists($fpath)){
    // Error
    require_once('404.php');
    die();
}

// Load the do=file
require_once($fpath);

// Nothing more comes in index.php

login.php

<?php

// Load template file header
require_once('header_template.php');


//
// Stuff that happens in login.php
// Checking if $_POST['username'] has valid chars then searching for it with pdo in db,
// password_verify() for $_POST['password'], etc...
//


// Load login's html template 
// (which includes the html form submitted to /login/)
require_once('login_template.php');

// Load template footer
require_once('footer_template.php');

.htaccess

To run with site.com/login/ etc instead of site.com?do=login

<IfModule mod_rewrite.c>
    RewriteEngine On
    RewriteCond   %{REQUEST_FILENAME} !-d
    RewriteCond   %{REQUEST_FILENAME} !-f
    RewriteRule   ^((?s).*)$ index.php?do=$1 [QSA,L]
</IfModule>
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Using GET for this is completely the wrong approach, your basically nuking your ability to properly use GET for what it's intended. Instead use the URI part of the URL. For example see this simple MVC router I made using URI instead of GET (as most major MVC framworks do) - the added benefit is you never have to touch HTACCESS except to remove index.php. To be clear the URI is the imaginary part of the URL www.example.com/index.php/controller/method/args so everything after index.php \$\endgroup\$ Mar 26 '19 at 20:16
  • \$\begingroup\$ @ArtisticPhoenix This comment was exactly what I was looking for :) I had a feeling that I'm doing something wrong, could you kindly post it as an answer please \$\endgroup\$
    – J. Doe
    Mar 26 '19 at 20:30
  • 1
    \$\begingroup\$ I'll post an answer explaining it in a few minutes. \$\endgroup\$ Mar 26 '19 at 20:31
2
\$\begingroup\$

Preface

I wouldn't use the query string of the URL at all.

The problem with using GET, is that GET is meant for other things such as bookmarkable search link etc. As you add more do stuff in your logic will become more and more complex. Harder and harder to keep track of what ever do does etc. There is no clear structure to it, no way to tell foo.php is a controller and bar.php is some other piece of code (like a DB class etc).

A better method is to use a MVC style router that does not affect the GET query string at all.

Another issue in your code is this:

 if(preg_match('/^[A-Za-z0-9_]+$/', $file_name) !== 1 OR ! file_exists($fpath)){

Which is ... muah ... ok. But it's the only thing preventing directory transversal attacks.

One more point is that while this may seem like the simplest method

if(isset($_GET['do'])){
    $file_name = rtrim($_GET['do'],'/');
}

You really have no control over what is being loaded. No way to know if that PHP file really should be used as a controller.

We can fix all these issues by using the URI part of the URL. For example take this URL

 www.example.com/index.php/user/login

The URI is the part after the index.php. We can very easly remove the index.php file (same as wordpress, or many MVC frameworks do).

So how do we do this:

Below I will post the full code for My Simple Router you can find here. This is a minimal example and not really meant for production use.

.htaccess (very similar or identical to what you'll find in wordpress etc.)

<IfModule mod_rewrite.c>
     RewriteEngine On

    # For sub-foder installs set your RewriteBase including trailing and leading slashes
    # your rewrite base will vary, possibly even being / if no sub-foder are involved
    RewriteBase /MISC/Router/

    RewriteCond %{REQUEST_FILENAME} !-f
    RewriteCond %{REQUEST_FILENAME} !-d
    RewriteRule ^(.*)$ index.php/$1 [L]
</IfModule>

index.php

<?php
require_once __DIR__.'/SimpleRouter.php';
SimpleRouter::route();

SimpleRouter.php

<?php
/**
 * A simple 1 level router
 * 
 * URL schema is http://example.com/{controller}/{method}/{args ... }
 * 
 * @author ArtisticPhoenix
 * @package SimpleRouter
 */
class SimpleRouter{

    /**
     * should be the same as rewrite base in .htaccess
     * @var string
     */
    const REWRITE_BASE = '/MISC/Router/';

    /**
     * path to controller files
     * 
     * @var string
     */
    const CONTOLLER_PATH = __DIR__.'/Controllers/';

    /**
     * route a url to a controller
     */
    public static function route(){
        //normalize
        if(self::REWRITE_BASE != '/'){
            $uri = preg_replace('~^'.self::REWRITE_BASE.'~i', '',$_SERVER['REQUEST_URI']);
        }

        $uri = preg_replace('~^index\.php~i', '',$uri);      
        $uri = trim($uri,'/');

        //empty url, like www.example.com
        if(empty($uri)) $uri = 'home/index';

        //empty method like www.example.com/home
        if(!substr_count($uri, '/')) $uri .= '/index';

        $arrPath = explode('/', $uri);

        $contollerName = array_shift($arrPath);
        $methodName = array_shift($arrPath);;
        $contollerFile = self::CONTOLLER_PATH.$contollerName.'.php';

        //require_once $contollerFile; //> when not autoloading, explode takes care of / in REQUEST_URI

        if(!file_exists($contollerFile)||!class_exists($contollerName)){
            self::error404($uri);
            return;
        }

        $Controller = new $contollerName();

        if(!method_exists($Controller, $methodName)){
            self::error404($uri);
            return;
        }

        if(!count($arrPath)){
            call_user_func([$Controller, $methodName]);
        }else{
            call_user_func_array([$Controller, $methodName], $arrPath);
        } 
    }

    /**
     * call error 404
     * 
     * @param string $uri
     */
    protected static function error404($uri){
        require_once self::CONTOLLER_PATH.'home.php';     
        $Controller = new home();
        $Controller->error404($uri);
    }
}

Contollers/home.php

<?php
/**
 * 
 * The default controller
 * 
 * @author ArtisticPhoenix
 * @package SimpleRouter
 */
class home{

    public function index($arg=false){
        echo "<h3>".__METHOD__."</h3>";
        echo "<pre>";
        print_r(func_get_args());
    }

    public function otherpage($arg){
        echo "<h3>".__METHOD__."</h3>";
        echo "<pre>";
        print_r(func_get_args());
    }

    public function error404($uri){
        header('HTTP/1.0 404 Not Found');
        echo "<h3>Error 404 page {$uri} not found</h3>";
    }

}

How it works

This is a decent amount of code, so the first thing is you'll probably want to change the RewriteBase to just / in both Htaccess and the Router class. On my Dev server it's located in www/MISC/Router/ and so I have to use those so that it routes properly. Otherwise I would have to setup a vHost and I am way to lazy for that.

This may seem way more complex then using GET etc. etc. But it's really quite simple, the main part is right here:

  $_SERVER['REQUEST_URI']

Which may contain index.php depending on the URL. So using my above example:

 www.example.com/index.php/user/login //REQUEST_URI = index.php/user/login

So in this case we remove index.php

 $uri = preg_replace('~^index\.php~i', '',$uri);   

Then after some more checks and what not we split this user/login into ['user','login']. Which if we go by the pattern at the top of the Router class.

  http://example.com/{controller}/{method}/{args ... }

This tells us that user is the "Controller" and the method is "login". So we look for that controller file:

 if(!file_exists($contollerFile)||!class_exists($contollerName)){

If your not using an autoloader you can split this, and require the file between:

  if(!file_exists($contollerFile)) self::error404($uri);

   require_once $contollerFile;

  if(!class_exists($contollerName)) self::error404($uri);

When using an autoloader the class_exists method will trigger autoloading of the Controller class.

In anycase there is little chance a URL like

    http://example.com/../../foobar.php

Will work because the controller would be .. with a method of .. and an argument of foobar.php.

In anycase in the above example we don't have a user controller, so lets add one now.

Contollers/user.php

class user{

    public function index(){
        echo "<h3>".__METHOD__."</h3>";
    }


    public function login(){
        echo "<h3>".__METHOD__."</h3>";
    }

}

So this www.example.com/index.php/user/login would basically go to the login method of this user controller class. If you just did www.example.com/index.php/user this would go to the index method above. And if you just did www.example.com/index.php/ it goes to home::index.

As you can see now we can really organize our code, instead of multiple files we can have multiple methods user::login, user::logout, user::profile, etc... Which keeps things neat and simple.

Any extra path parts such as www.example.com/index.php/user/login/foo would pass foo as the first argument to user::login('foo') and so on for any additional arguments. So instead of losing $_GET we gain the ability to send extra information along with the URL itself, all the wile leaving $_GET to do what it was intended for.

With the rewriting we can actually set all our links up without the index.php so you would just omit that when creating navigation links etc.., and the rewrite rule will take care of it. The really big advantage is once it's setup you can just add new controllers or methods in and that is the extent of the changes you need to make for it to work.

Summery

This solves a lot of the above mentioned problems, because you have total control over what is a Controller and what is not. There is 100% no chance for directory transversal (if autoloading) because the path is never used, auto loading uses the Class Name/Namespace, not the file name. Even without autoloading, by it's very nature it will split the paths on the directory separator. So the protection is inherent, instead of added as an after thought.

The only real issue this type of router has it that you are tied to a naming convention for your URL and Controllers, but you have that anyway. A way around that is the next step up which is an "event" based router, where you would subscribe to the request event. This would let you run any function or callable as the endpoint of any URL, and break this dependence on the file system.

You can obviously go much deeper then this, such as having sub-folders in the Controller folder etc.

But this is just a simple example, I hope that helps explain the basics.

\$\endgroup\$
8
  • \$\begingroup\$ Would not be better to just use a Alias? \$\endgroup\$
    – mario
    Mar 26 '19 at 21:12
  • \$\begingroup\$ Would not be better to just use a Alias - I don't know what you mean, but this is just a simple example that doesn't transform the Class name or methods etc. It was about the smallest I could make this with still having good fallback protection for variation in the URL> \$\endgroup\$ Mar 26 '19 at 21:17
  • \$\begingroup\$ The problem is as soon as you do this in mod rewrite RewriteRule ^((?s).*)$ index.php?do=$1 [QSA,L] - your forever changing the GET part of the request. This makes it difficult to use it for what it's intended such as search arguments etc... Essentially your taking the responsibility of the URL (routing to resources) and infecting the Query string with it. \$\endgroup\$ Mar 26 '19 at 21:19
  • \$\begingroup\$ serverfault.com/a/362041 He could point the folder: /user/ to application/actions/. So this is another way to load files outsite public_html, and this don't really needs a GET request. he could use this url: https://example.com/user/login.php. \$\endgroup\$
    – mario
    Mar 26 '19 at 21:37
  • 1
    \$\begingroup\$ Sure, glad I could help expand your understanding. \$\endgroup\$ Mar 26 '19 at 21:56
2
\$\begingroup\$

You can simplify your regex pattern and you might like to combine your validation with the preparation step. The following pattern is just like yours except it allows a single trailing slash and omits it from the capture group.

if (isset($_GET['do']) && preg_match('~^(\w+)/?$~', $_GET['do'], $valid)) {
    $file_name = $valid[1];
} else {
    $file_name = "home_page";
}

$fpath = FILESPATH . "{$file_name}.php";

if (!file_exists($fpath)) {
    // Error
    require_once('404.php');
    die();
}

It doesn't make sense to valid home_page with your pattern, you know it will pass every time. I suppose it could be possible that your home_page file may not exist in the directory.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.