Safe Dynamic Includes
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...
On Sat 19th Aug 2006 @ 14:54, Lea said:
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...
On Sat 19th Aug 2006 @ 15:51, Xeronia said:
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.
On Sat 19th Aug 2006 @ 17:07, Corinne said:
The pootato tut I understand, this, I don't but I'll try it out later.
On Sat 19th Aug 2006 @ 17:10, Jem said:
@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.
On Sat 19th Aug 2006 @ 17:38, Corinne said:
lol. I know. I'm about 2/3rds through in understanding it.
On Sat 19th Aug 2006 @ 17:40, Jem said:
I can explain each line individually if it'd help?
On Sat 19th Aug 2006 @ 17:49, Corinne said:
I'm close, but I can figure out the last part. I'm not sure how to include more pages.
On Sat 19th Aug 2006 @ 17:52, Jem said:
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?
On Sat 19th Aug 2006 @ 17:54, Corinne said:
Yea, I think I've got it now.
On Sat 19th Aug 2006 @ 21:03, Jenny said:
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?
On Sat 19th Aug 2006 @ 21:06, Jem said:
@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.
On Sat 19th Aug 2006 @ 21:24, Corinne said:
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?
On Sun 20th Aug 2006 @ 00:45, Bubs said:
Hey... you may already know but your site is an entree on Design Snack
On Sun 20th Aug 2006 @ 18:28, kachii said:
Wizard's first rule - People are stupid. Or should that be coder's first rule?
On Mon 21st Aug 2006 @ 14:57, Stephanie said:
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.
On Mon 21st Aug 2006 @ 17:09, Claire said:
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
On Tue 22nd Aug 2006 @ 09:13, Jasper said:
That easy huh?
Some things are simple and priceless...
On Wed 23rd Aug 2006 @ 10:56, Melissa said:
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!
On Fri 25th Aug 2006 @ 02:45, Xeronia said:
That is a lot of very helpful code!
On Fri 25th Aug 2006 @ 08:53, Adrianne said:
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!).
On Thu 31st Aug 2006 @ 13:39, Clara said:
I found your post thanks to my tracker and I'm glad I checked it! I will redirect users to this snippet, thanks!
On Sun 17th Sep 2006 @ 16:41, Hillarie said:
Thank you SO much! I don't want unsafe includes on my site. You've saved a lot of sites from being hacked!
On Fri 29th Sep 2006 @ 17:45, Senora said:
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
On Thu 5th Oct 2006 @ 17:11, la Pepa said:
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!
On Sat 9th Jun 2007 @ 12:11, Six said:
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?