Page 1 of 1

What is wrong with this code?

Posted: Tue Nov 09, 2004 7:00 pm
by MonkeyManx
I have been using this code on all of my websites, but recently it no longer works (because of php upgrade?)

Code: Select all

<?php

// this is if the variable isnt specified, itll set the variable to your intro/main page.

if (!$pageid) {
$pageid = "main";
$include = $pageid . ".htm";
}
else {
$include = $pageid . ".htm";
}
if (is_file($include) == "1") {
        include $include;
}
else {
// if the file doesnt exist, itll include the error page
include "404.htm";
}

?>

Posted: Tue Nov 09, 2004 7:08 pm
by Weirdan
have a read about register_globals

and use [php_man]basename[/php_man] to remove possible file path passed by the user. As of PHP 5.0 ftp wrapper supports stat functionality, so if I pass "ftp://my.server/path/to.file" your script would include ftp://my.server/path/to.file.htm and execute it on your server as if it was local file

Posted: Tue Nov 09, 2004 7:40 pm
by josh
heh, that script isnt too secure... as weirdman said... some one can force php code to be executed on your server and pretty much delete all your files if they wanted.

Posted: Tue Nov 09, 2004 7:57 pm
by MonkeyManx
What if I used it like this:

$include = "path/on/server/$pageid.htm";

Posted: Tue Nov 09, 2004 8:01 pm
by John Cartwright
Weirdan wrote:have a read about register_globals

and use [php_man]basename[/php_man] to remove possible file path passed by the user. As of PHP 5.0 ftp wrapper supports stat functionality, so if I pass "ftp://my.server/path/to.file" your script would include ftp://my.server/path/to.file.htm and execute it on your server[/b] as if it was local file

Posted: Tue Nov 09, 2004 8:02 pm
by Weirdan
still

Code: Select all

page?pageid=..%2f..%2f..%2f..%2fetc%2fpasswd
would disclose sensitive information

Posted: Tue Nov 09, 2004 8:43 pm
by josh
MonkeyManx wrote:What if I used it like this:

$include = "path/on/server/$pageid.htm";
That would prevent a user from includeing something off their site but some one could still do ../../../../password or something and read out some of your own files

get rid of any characters that are non a-z or 0-9, do the path/on/server thing too

a switch is always more secure then putting a variable into an include(); ... just depends on how many files there are to include, obviously you would not write a switch for 300 files..

also do the following:

Code: Select all

<?php
$file=str_replace("badstuff", "goodstuff", $file);
$file="path/whatever/$file.htm";
if (file_exists($path)) {
 include ($path);
} else {
echo "Error";
}
?>

Posted: Tue Nov 09, 2004 8:59 pm
by Weirdan
Here is another option:

Code: Select all

$include_root = '/path/to/your/site/modules/'; // user can include anything under this dir and its subdirs
$module = realpath($include_root . $_GET['module']);
if( ( substr($module, 0, strlen($include_root) != $include_root ) ) || !file_exists($module) )
     die("Break-in attempt.");
else
     include $module;