[php] HELP me with "include($_GET['site']);"

[scofield]

Hey everyone!
Can one help me to make the code secure?

Code: [Select]

<?php
               if($_GET['site'] == "")
               {
                  chdir("news");
                  $show = "main";
                  include("shownews.php");
                  chdir("..");
               }
               else
               {
                  if(file_exists($_GET['site']))
                  {
                     if(strstr($_GET['site'], "http://"))
                     {
                        echo "External files cannot be integrated.";
                     }
                     else
                     {
                        include($_GET['site']);
                     }
                  }
                  else
                  {
                     echo "FILE NOT FOUND";
                  }
               }
            ?>

[Stackprotector]

sure,  you need to secure your GET input,   what is the purpose of this script??, as a file manager??

[ande]

I really do recommend not doing it this way. Do a case/if statement or have an array with allowed words/pages and do an if(in_array()) thingy, that would guarantee security.

Alternatively, read up on the RFI tutorial I wrote a while back, I believe there were someone who made a comment about filtering out "dangerous" stuff and do the include() directly. But there is always some things one forgets to filter out, and on top of that there are often new bypass methods discovered.

[scofield]

the first line include the news-script (shownews.php), the other lines are normal php-files (content)

hm....... can i have an exemple?

sry i'm a noob in this... a ananymous guy tell me this

[ande]

Thing is, its easy to say "this is allowed, this is allowed and this is allowed" than to say everything that is not allowed. In most cases, the not-allowed list will be infinitely long and the allowed list will be 2-20 lines. Therefore, its easier to do:

(If statement):

Code: [Select]

if($_GET['site']=="page")
{
     include($_GET['site']);
}
elseif($_GET['site']=="page2")
{
     include($_GET['site']);
}
else
{
     include("home.php");
}

(Case statement):

Code: [Select]

switch($_GET['site'])
{
  case "page":
  include($_GET['site']);

  case "page2":
  include($_GET['site']);

  default:
  include("home.php");
}

(In_array()):

Code: [Select]

$sites = array("page", "page2");

if(in_array($_GET['site'], $sites))
{
     include($_GET['site'];
}


Also remember to do isset($_GET['parameter']) before checking for values. Switch() might also give an error if you give it an array, so might want to check if the GET parameter have been tampered into an array (http://site.com/?site[]=a). Just do a is_array() before the switch().

[Stackprotector]

if you are not using server side scripting in the included files (php,asp etc.)  try using echo file_get_contents(filename);

[scofield]

hm... okay, i must create an list for my content-files (*.php), but i have many content-files and the list where a long-long-list... is there an other way to include secure?

btw: my url-link are at the moment so: http://xxxxx.com/?site=demo.php ... Can I maintain the left path so?

[ande]

hm... okay, i must create an list for my content-files (*.php), but i have many content-files and the list where a long-long-list... is there an other way to include secure?

btw: my url-link are at the moment so: http://xxxxx.com/?site=demo.php ... Can I maintain the left path so?

There is no easy way doing this. Use less files, unless you are making something on the scale of vBulletin you wont need more than a few files. And you should not have the .php part in the URL, just plain nasty

How many files you got? Unless its like 100+, the list is not long.