Dynamic Includes

**Ancyru** · July 10th, 2004, 10:36 PM

Hey guys, I'm not to confident with PHP but here's my shot at it.

Code:

<?php
if($x) {
  if(file_exists("$x.php") {
    include("$x.php");
  } else {
    include("error.php");
} elseif(!$x) {
    include("main.php");
}
?>

Is there anything wrong with it or could it be improved?

H · July 10th, 2004, 10:42 PM

That's fine and all, but here's what I would use:

<?

(include("$x.php")) OR include("error.php");
?>

Hope it works/helps.

**Ancyru** · July 10th, 2004, 10:45 PM

Could you explain why you would use that?

H · July 10th, 2004, 11:56 PM

Well, it's less code and does pretty well the exact same thing. File size is reduced slightly. Just preference really.

Wolfy · July 14th, 2004, 10:17 AM

A few words about security (yes; I know I'm a nag).

Doing dynamic includes of this nature is very risky - particularly if you are also allowing your users to upload files elsewhere on the page. Consider you have a forum that allows users to upload an avatar or attach files to their posts and this script fails to adequately check the file type of the upload.

Then a malicious user might create a file "exploit.jpg" that was actually a php file containing some malicious code. e.g.

Code:

<?php
echo "<strong>H4H D00D UR 73H L4M3R!!!11!!<strong>";
?>

This code would normally not be able to be run due to the extension being linked to the image/jpeg MIME-type but PHP does not check these things when using include or require - so the code could be run by calling http://yourdomain.com/yourscript.php...rs/exploit.jpg

The example I have given would just embarass you (and good grammar) by calling you 73H L4M3R; a real exploit might deliberately trash your site or overload the server disrupting your own service and others.

It should be assumed that any user input into forms is unsafe. Your script could either check that the include in $x points to a directory with the appropriate permissions.

Code:

<?php
if($x) {
  if (strpos($x,"/mysafedir/")!=1) {
    include("stophackingme.php");
  }
  else if(file_exists("$x.php") {
    include("$x.php");
  } else {
    include("error.php");
} elseif(!$x) {
    include("main.php");
}
?>

The additional lines check to make sure that $x starts with the path to a directory with read-only permissions set on it. Please note this is also insufficient (though better) as $x could contain a string such as "/mysafedir/../avatars/exploit.jpg". If you must insist on passing the filename of the include the proper way to go about it would be to explode the entire string and check the path resolves to a secure directory.

The most secure way to implement this is to know the pages you need before hand and use a method similar to that shown in this helpdesk howto:
http://desk.surpasshosting.com/index...e1e564639e2e44

-wolf

GoBarking · July 15th, 2004, 2:24 AM

Wolfy, all I can say is WOW! Thanks for the fantastic info! Your so right about using using include or require. I totally see it now. (My light bulb just dimly lit!)
Thank you!
~VH~

**SmartGuy** · July 15th, 2004, 2:06 PM

Actually, that's still dangerous. I could put in index.php?x=../file.php and open file.php in the parent directory. index.php?x=../../filephp and the same goes for the parent's parent directory.

I suggest using a switch. It's the safest way to do what you want. For each page that you have to add a new line just like all the others, with the correct name and file.

PHP Code:


			
<?php

switch($_GET["x"]){

    case "news": include("news.php"); break;  // index.php?x=news

    case "games": include("games.php"); break;  // index.php?x=games

    case "links": include("links.php"); break;  // index.php?x=links

    default: include("home.php"); break;  // index.php

}

?>

GoBarking · July 15th, 2004, 6:53 PM

This is getting better and better! Thanks guys!

Wolfy · July 15th, 2004, 7:45 PM

In my defense ... I did say in my original post that the method I had just proposed was insufficient for precisely the reason you posit and linked to a helpdesk article which used the case method.

-wolf

July 10th, 2004, 10:36 PM	#1 (permalink)
Ancyru L'Awesome Advocate Super #1 Joined in May 2004 Lives in .au Hosted on Mango 2,423 posts Gave thanks: 1 Thanked 5 times	Dynamic Includes Hey guys, I'm not to confident with PHP but here's my shot at it. Code: <?php if($x) { if(file_exists("$x.php") { include("$x.php"); } else { include("error.php"); } elseif(!$x) { include("main.php"); } ?> Is there anything wrong with it or could it be improved? __________________ When I get sad, I stop being sad, and be AWESOME instead. True story.

July 10th, 2004, 10:45 PM	#3 (permalink)
Ancyru L'Awesome Advocate Super #1 Joined in May 2004 Lives in .au Hosted on Mango 2,423 posts Gave thanks: 1 Thanked 5 times	Could you explain why you would use that? __________________ When I get sad, I stop being sad, and be AWESOME instead. True story.

July 14th, 2004, 10:17 AM	#5 (permalink)
Wolfy Registered User Seasoned Poster Joined in Jul 2004 31 posts Gave thanks: 0 Thanked 0 times	A few words about security (yes; I know I'm a nag). Doing dynamic includes of this nature is very risky - particularly if you are also allowing your users to upload files elsewhere on the page. Consider you have a forum that allows users to upload an avatar or attach files to their posts and this script fails to adequately check the file type of the upload. Then a malicious user might create a file "exploit.jpg" that was actually a php file containing some malicious code. e.g. Code: <?php echo "<strong>H4H D00D UR 73H L4M3R!!!11!!<strong>"; ?> This code would normally not be able to be run due to the extension being linked to the image/jpeg MIME-type but PHP does not check these things when using include or require - so the code could be run by calling http://yourdomain.com/yourscript.php...rs/exploit.jpg The example I have given would just embarass you (and good grammar) by calling you 73H L4M3R; a real exploit might deliberately trash your site or overload the server disrupting your own service and others. It should be assumed that any user input into forms is unsafe. Your script could either check that the include in $x points to a directory with the appropriate permissions. Code: <?php if($x) { if (strpos($x,"/mysafedir/")!=1) { include("stophackingme.php"); } else if(file_exists("$x.php") { include("$x.php"); } else { include("error.php"); } elseif(!$x) { include("main.php"); } ?> The additional lines check to make sure that $x starts with the path to a directory with read-only permissions set on it. Please note this is also insufficient (though better) as $x could contain a string such as "/mysafedir/../avatars/exploit.jpg". If you must insist on passing the filename of the include the proper way to go about it would be to explode the entire string and check the path resolves to a secure directory. The most secure way to implement this is to know the pages you need before hand and use a method similar to that shown in this helpdesk howto: http://desk.surpasshosting.com/index...e1e564639e2e44 -wolf

July 15th, 2004, 2:06 PM	#7 (permalink)
SmartGuy Surpass Fan Excelling Contributor Joined in Jan 2004 Lives in Clinton, Massachusetts Hosted on Serenity x.x.40.51 994 posts Gave thanks: 0 Thanked 0 times	Actually, that's still dangerous. I could put in index.php?x=../file.php and open file.php in the parent directory. index.php?x=../../filephp and the same goes for the parent's parent directory. I suggest using a switch. It's the safest way to do what you want. For each page that you have to add a new line just like all the others, with the correct name and file. PHP Code: `<?php switch($_GET["x"]){ case "news": include("news.php"); break; // index.php?x=news case "games": include("games.php"); break; // index.php?x=games case "links": include("links.php"); break; // index.php?x=links default: include("home.php"); break; // index.php } ?>`

Thread Tools	Search this Thread
Show Printable Version Email this Page	Search this Thread: Advanced Search
Rate This Thread
Rate This Thread:

July 10th, 2004, 10:42 PM	#2 (permalink)
H after g, before i Resident. Joined in Jul 2004 Lives in N,BC,CA 8,087 posts Gave thanks: 48 Thanked 131 times	That's fine and all, but here's what I would use: <? (include("$x.php")) OR include("error.php"); ?> Hope it works/helps.

July 10th, 2004, 11:56 PM	#4 (permalink)
H after g, before i Resident. Joined in Jul 2004 Lives in N,BC,CA 8,087 posts Gave thanks: 48 Thanked 131 times	Well, it's less code and does pretty well the exact same thing. File size is reduced slightly. Just preference really.

July 15th, 2004, 2:24 AM	#6 (permalink)
GoBarking Registered User Fresh Surpasser Joined in Jul 2004 8 posts Gave thanks: 0 Thanked 0 times	Wolfy, all I can say is WOW! Thanks for the fantastic info! Your so right about using using include or require. I totally see it now. (My light bulb just dimly lit!) Thank you! ~VH~

July 15th, 2004, 6:53 PM	#8 (permalink)
GoBarking Registered User Fresh Surpasser Joined in Jul 2004 8 posts Gave thanks: 0 Thanked 0 times	This is getting better and better! Thanks guys!

July 15th, 2004, 7:45 PM	#9 (permalink)
Wolfy Registered User Seasoned Poster Joined in Jul 2004 31 posts Gave thanks: 0 Thanked 0 times	In my defense ... I did say in my original post that the method I had just proposed was insufficient for precisely the reason you posit and linked to a helpdesk article which used the case method. -wolf

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)