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…
19 Aug 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… :)
19 Aug 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.
19 Aug at 4:07 pm
The pootato tut I understand, this, I don’t but I’ll try it out later.
19 Aug 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.
19 Aug at 4:38 pm
lol. I know. I’m about 2/3rds through in understanding it.
19 Aug at 4:40 pm
I can explain each line individually if it’d help? :)
19 Aug at 4:49 pm
I’m close, but I can figure out the last part. I’m not sure how to include more pages.
19 Aug 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?
19 Aug at 4:54 pm
Yea, I think I’ve got it now.
19 Aug 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?
19 Aug 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.
19 Aug 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?
19 Aug at 11:45 pm
Hey… you may already know but your site is an entree on Design Snack ;)
20 Aug at 5:28 pm
Wizard’s first rule – People are stupid. Or should that be coder’s first rule?
21 Aug 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.
21 Aug 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 ;)
22 Aug at 8:13 am
That easy huh? :) Some things are simple and priceless…
23 Aug 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!
25 Aug at 1:45 am
That is a lot of very helpful code!
25 Aug 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!).
31 Aug 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!
17 Sep 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!
29 Sep 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
05 Oct 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!
09 Jun 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?