Safe Dynamic Includes

25 comments

I was directed to pootato.org’s Dynamic Inclusion tutorial yesterday by somebody who has been using it, and as a result have been “hacked” — index page defaced as well as a fake banking website/etc put on her web space. This girl could potentially land in serious trouble, depending on how forgiving her hosts are, because of someone else’s mistakes (and you see now why I won’t shut up about security?)

The pootato.org tutorial has one major flaw — although the creator has tried to play it safe using the basename() function, they’ve continued to use the possibly tainted $x variable (which, I must add, won’t work unless register_globals is enabled). As I said yesterday: if you don’t do some sort of validation of the URL you’re including, all sorts of files including malicious scripts on other servers, sensitive data from below-root folders, etc can be injected and exploited through that one simple line of code.

The best way ’round this is to use the safer variable $page, the one that has been “cleaned”. Still, the problem then exists that people using directories to store their files will not be able to use the code (and those with register_globals disabled can’t either). It was this problem with directories that forced me to write the following:

<?php if (isset($_GET['x'])) {
   if (strpos($_GET['x'], "/")) {
      $dir = substr(str_replace('..', '', $_GET['x']), 0, strpos($_GET['x'], "/")) . "/";
      $file = substr(strrchr($_GET['x'], "/"), 1);
      include($dir.$file.".html");

   } else {
      include(basename($_GET['x']).".html");
   }
} else {
   include("default-page.html");
} ?>

The code checks for the existance of ‘x’, if there is no ‘x’ appended to the URL, it includes the default page. If ‘x’ is defined (i.e. index.php?x=foo), it checks for a forward slash, which would indicate a directory. If a “/” is there it grabs the directory name (stripping out “..” to prevent people from including things below the current directory) and filename to display and includes the relevant file. This only works up to one directory deep — preventing URLs from being included (http:/ will be seen as a directory and will ‘break’). If no directory is found, it will attempt to include the file based on the query string with a .html extension.

The snippet of code can be further improved too. By checking for existance of the files with file_exists() before they’re included, we can rely on our default page and avoid ugly PHP errors:

<?php if (isset($_GET['x'])) {
   if (strpos($_GET['x'], "/")) {
      $dir = substr(str_replace('..', '', $_GET['x']), 0, strpos($_GET['x'], "/")) . "/";

      $file = substr(strrchr($_GET['x'], "/"), 1);
      if (file_exists($dir.$file.".html")) {
         include($dir.$file.".html");
      } else {
         include("default-page.html");

      }
   } else {
      if (file_exists(basename($_GET['x']).".html")) {
         include(basename($_GET['x']).".html");
      } else {
         include("default-page.html");

      }
   }
} else {
   include("default-page.html");
} ?>

So there you have a much safer method, which supports files in single directories, and will show a default page instead of a mess of errors. Of course, the even safer option would be to avoid using these “dynamic includes” altogether so that you’re not reliant on user input to deliver your pages…

Jem Turner jem@jemjabella.co.uk +44(0)7521056376

25 comments so far

  1. Lea said:
    On August 19, 2006 at 1:54 pm

    I think it had been stated long long ago that the method with the x= think could be unsafe… I’m not a PHP genius at all, but I’m really happy you give your visitors some advice about security because my host has been hacked recently and I wouldn’t want to make her go through the same disaster again… :)

  2. Xeronia said:
    On August 19, 2006 at 2:51 pm

    I don’t understand most of the code there, as I don’t know a lot of PHP. I think that the hotess will be tolerant. Most are. I can tolerate your security rants now.

  3. Corinne said:
    On August 19, 2006 at 4:07 pm

    The pootato tut I understand, this, I don’t but I’ll try it out later.

  4. Jem said:
    On August 19, 2006 at 4:10 pm

    @Corinne: that’s because this isn’t a tutorial, it’s a piece of code. :) Follow the pootato tutorial and then simply replace the insecure PHP from that with the secure code from here. That is of course, unless you want to use the insecure code but why anyone would choose to do that, I do not know.

  5. Corinne said:
    On August 19, 2006 at 4:38 pm

    lol. I know. I’m about 2/3rds through in understanding it.

  6. Jem said:
    On August 19, 2006 at 4:40 pm

    I can explain each line individually if it’d help? :)

  7. Corinne said:
    On August 19, 2006 at 4:49 pm

    I’m close, but I can figure out the last part. I’m not sure how to include more pages.

  8. Jem said:
    On August 19, 2006 at 4:52 pm

    Just create the pages (as .html – otherwise you must change .html to .php or whatever in the code) and then link to them in your sidebar/navigation as index.php?x=page where ‘page’ is the name of the saved file, without the extension. E.g. if you’ve got a file called about-me.html you’d link to it as index.php?x=about-me If you want to link to a file in a directory it’d be: index.php?x=directory/filename Was that what you needed to know, or was it something else?

  9. Corinne said:
    On August 19, 2006 at 4:54 pm

    Yea, I think I’ve got it now.

  10. Jenny said:
    On August 19, 2006 at 8:03 pm

    Hm, would it be much safer if you defined which pages went where? Like, if you set “me” to go to about-me.html et cetera, so if the user went somewhere that wasn’t defined they’d get a 404?

  11. Jem said:
    On August 19, 2006 at 8:06 pm

    @Jenny: If the user goes somewhere that isn’t defined they’ll get the default page anyway (which could be set to the 404 page.) It could be set to only allow predefined values but then every time you want to add a page you’d need to update the php.

  12. Corinne said:
    On August 19, 2006 at 8:24 pm

    I took a break, but as you can see, I got it to work: http://jarlylove.com/test (the default page is main.php and the other page is credit.php). /?x=main and /?x=credit. But, how do you set the title?

  13. Bubs said:
    On August 19, 2006 at 11:45 pm

    Hey… you may already know but your site is an entree on Design Snack ;)

  14. kachii said:
    On August 20, 2006 at 5:28 pm

    Wizard’s first rule – People are stupid. Or should that be coder’s first rule?

  15. Stephanie said:
    On August 21, 2006 at 1:57 pm

    Mmm, snackalicious Jemjabella ;) Can I say that the orange for your <code> tags is a bit hard to read in long lines? I can read it better if I increase the text size, but otherwise it makes my astigmatism go haywire ;P Now for something relevant: That definitely looks much safer than what’s at pootato… personally, I’m all for the simple PHP includes because nothing bad can really happen that way. At least, Michael hasn’t yelled at me for using them yet (and he’s a security freak), so that must mean they’re pretty okay, heh.

  16. Claire said:
    On August 21, 2006 at 4:09 pm

    Jem, I just wanted to say thank you for all these “security-based” post that you’re putting out at the moment. When I was taught PHP, not much focus was given on security…minor input validation was featured, but nothing like this. Hence, when I tried to write my own script a year or so ago, I got hacked royally and nearly lost my site account. Now, I can re-learn PHP in a more secure way and atleast think twice, if not a dozen times, before using a script I’ve put together myself. Cheers for filling in what my syllabus missed out ;)

  17. Jasper said:
    On August 22, 2006 at 8:13 am

    That easy huh? :) Some things are simple and priceless…

  18. Melissa said:
    On August 23, 2006 at 9:56 am

    When php turns faulty Jem is always to the rescue. I’m sure this post alone will help a ton of people. I know it’d help me if I used that method!

  19. Xeronia said:
    On August 25, 2006 at 1:45 am

    That is a lot of very helpful code!

  20. Adrianne said:
    On August 25, 2006 at 7:53 am

    Maybe it’s just me but I never really understood dynamic includes. I think I’m comfortable with regular PHP includes, but after reading your tutorial on Tutorialtastic now it’s making me ponder that I should try dynamic includes in the future (with your secure snippets and all the package!).

  21. Clara said:
    On August 31, 2006 at 12:39 pm

    I found your post thanks to my tracker and I’m glad I checked it! I will redirect users to this snippet, thanks!

  22. Hillarie said:
    On September 17, 2006 at 3:41 pm

    Thank you SO much! I don’t want unsafe includes on my site. You’ve saved a lot of sites from being hacked!

  23. Senora said:
    On September 29, 2006 at 4:45 pm

    Hi, Really cool comments board…. Will probably use it on my site when it is fimished. I read some of your reviews, well….I will never tell you the name of my site…wow…You are hard!! Thanks, SMA

  24. la Pepa said:
    On October 5, 2006 at 4:11 pm

    hello, this works just fine, thank you very much! Is there a way to use this code withour reloading the page? I’ve got a javascript dynamic menu (included in the index.php), but each time I select a menu option reloads the page and doesn’t stay in the active link… thanks anyway!

  25. Six said:
    On June 9, 2007 at 11:11 am

    Jem, I understand what you’ve written above, but what I don’t understand is why someone would want to use dynamic includes? What is the purpose/benefit of using them? Could you please explain?