is my code subject to Remote File Inclusion RFI ?

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
vin_akleh
Forum Commoner
Posts: 53
Joined: Sat Feb 14, 2009 10:26 am

is my code subject to Remote File Inclusion RFI ?

Post by vin_akleh »

Code: Select all

if (isset($_REQUEST["page"]))
        {
            include $_REQUEST["page"].".php";
        }
        if (isset($_REQUEST["logout"]))
        {
            include "logout.php";
        }
well is it? if so what is the solution?
thanks
User avatar
flying_circus
Forum Regular
Posts: 732
Joined: Wed Mar 05, 2008 10:23 pm
Location: Sunriver, OR

Re: is my code subject to Remote File Inclusion RFI ?

Post by flying_circus »

It depends on your php settings, but most likely yes. It certailly allows maipulation of local file inclusion.

The solution is to validate your user input.
http://en.wikipedia.org/wiki/Remote_File_Inclusion

Personally, I would NEVER use $_REQUEST.
vin_akleh
Forum Commoner
Posts: 53
Joined: Sat Feb 14, 2009 10:26 am

Re: is my code subject to Remote File Inclusion RFI ?

Post by vin_akleh »

what php settings could you specify how to do it? or maybe show me the code or a link to fix this problem
vin_akleh
Forum Commoner
Posts: 53
Joined: Sat Feb 14, 2009 10:26 am

Re: is my code subject to Remote File Inclusion RFI ?

Post by vin_akleh »

is this the solution??
of course the $path should be preseted

Code: Select all

        if (isset($_REQUEST["page"]))
        {
            $page=$_REQUEST['page'];
            if(file_exists("$path/$page.php"))
            include "*/".$_REQUEST["page"].".php";
        }
        if (isset($_REQUEST["logout"]))
        {
            $logout=$_REQUEST['logout'];
            if(file_exists("$path/$logout.php"))
            include "logout.php";
        }
User avatar
flying_circus
Forum Regular
Posts: 732
Joined: Wed Mar 05, 2008 10:23 pm
Location: Sunriver, OR

Re: is my code subject to Remote File Inclusion RFI ?

Post by flying_circus »

You are not validating user input. $_REQUEST pulls a value from $_GET, $_POST, and $_COOKIE in some order. The order is set in the php.ini and will probably work, provided it doesnt change, however, this isnt best practice. If you are expecting to pull page from the querystring such as: "http://www.example.org?page=logout", but I set a cookie and send it to you with a key/value pair of "page=not_what_you_expected", then your program will understand $page as "not_what_you_expected". Use $_GET, $_POST, or $_COOKIE explicitly, and try to never use $_REQUEST.

Depending on your site's architecture, a simple script such as the following might work. At the very least, if your program gets "not_what_you_expected", you dont run the risk of including the incorrect file. There are better and probably faster ways to handle this, but its a start and atleast your accepted value's are white-listed.

Code: Select all

<?php
  /**
  * $_GET['page'] pulls page from the URL querystring.
  * $_POST['page'] pulls page from form post values.
  * $_COOKIE['page'] pulls page from a cookie with key "page".
  * 
  * KNOW where your data is coming from.  (Don't use $_REQUEST)
  */
  # Fetch page
    $page = (isset($_GET['page'])) ? $_GET['page'] : '';
    
  # Define absolute path
    $absolute_path = '/var/www/';
    
  # Include your page
    switch($page)
    {
      case 'my_account':
        include_once($absolute_path . 'my_account.php');
        break;
      case 'logout':
        include_once($absolute_path . 'logout.php');
        break;
      default:
        include_once($absolute_path . 'home.php');
        break;
    }
?>
Also see:
http://www.php.net/manual/en/filesystem ... -url-fopen
http://www.php.net/manual/en/filesystem ... rl-include
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: is my code subject to Remote File Inclusion RFI ?

Post by kaisellgren »

vin_akleh wrote:

Code: Select all

if (isset($_REQUEST["page"]))
        {
            include $_REQUEST["page"].".php";
        }
        if (isset($_REQUEST["logout"]))
        {
            include "logout.php";
        }
well is it? if so what is the solution?
thanks
You have a remote file inclusion threat in your application, and a local file inclusion vulnerability. You can't trust any data the client sends you.

A way better approach would be:

Code: Select all

$allowedPages = array('home','downloads','contact');
if (!in_array($_GET['page'], $allowedPages)) // Is the page allowed to be accessed?
 $_GET['page'] = 'home';
include($_GET['page'].'.php');
If you search this forum for remote file inclusion you should find pretty much information.
Post Reply