Page 1 of 1

Need Some Advice

Posted: Fri Oct 15, 2010 12:41 am
by myister
Ok so I am creating this project management system for myself and and everything is going good.

I need to know if there is another way to do this or if there is a way to get around this.

I am kinda trying this new simple Idea of a template system using $_GET.

So what I did was made my index page and wrote

Code: Select all

<?php $empty=strlen($_GET['page']); 
if ($empty==0) 
{ 
header ("Location: 404error.php"); 
} 
// Now that the url is clean lets make sure that the file exsists and if not send them to the 404 page........
$page = $_GET['page'];
$urlcheck = $page.".php";
if (!file_exists($urlcheck)) {
    header("Location: 404error.php");
} 

$pagetitle = $page;
?>
Works awesome but for the fact that any of the files that I am including into the index file are unprotected because i can use sessions on the or it will give an error about sessions already being started. My main link is
localhost/administrator/index.php?page=main
When I direct link to the main.php file it will open in the browser since it does not use sessions. So I added in main.php the following

Code: Select all

<?php $empty=strlen($_GET['page']); 
if ($empty==0) 
{ 
header ("Location: 404error.php"); 
} 
// Now that the url is clean lets make sure that the file exsists and if not send them to the 404 page........
$page = $_GET['page'];
$urlcheck = $page.".php";
if (!file_exists($urlcheck)) {
    header("Location: 404error.php");
} ?> 
as far as I know it is working. When I go to the file directly it does do work but when called in the include it see's itself and then displays.

How secure could this be. I imagine it aint very at all but it is working!!!?


EDIT: Ok so it does NOT work. ANYONE have any suggestions on how to secure the included files?

Re: Need Some Advice

Posted: Fri Oct 15, 2010 1:46 am
by myister

Code: Select all

<?phpif(!isset($_SESSION['myusername'])){

// if the User is NOT logged in we will redirect him back to the login page first then back to here after they have entered the correct username and password
header("Location: login.php");die;
}?>
Stopped all direct access and still let the script use read the file. So far so good

Re: Need Some Advice

Posted: Fri Oct 15, 2010 10:55 am
by timWebUK
DON'T take page names through GET... ever!

People can use directory traversal, if there's an upload script, execute arbitrary code. Access your servers passwd file, etc. List is endless. DO NOT do it this way.

Use a whitelist array of keywords, and associate those with file names.

Code: Select all

$whitelist = {'main', 'home', 'help'};

if(in_array($_GET['i'])){

switch($_GET['i'])
{
case 'main':

//do this
break;
}

}else
{

// 404

}

Re: Need Some Advice

Posted: Fri Oct 15, 2010 1:06 pm
by VladSun
Always put an exit() call after having a header('Location ... ') call. Otherwise redirecting could be (forced to be) ignored by the client.

Re: Need Some Advice

Posted: Fri Oct 15, 2010 1:10 pm
by John Cartwright
@timWebUK: A switch statement is essentially a whitelist already, so there is no use for having an array of accepted values, then have to repeat a case for those values. Then, you can use the default case to catch invalid values.

Re: Need Some Advice

Posted: Mon Oct 18, 2010 10:15 am
by myister
Ok so using $_GET is bad if you leave it open to whatever for input.
I understand the concept to using an array to only let certain titles to pass through.

This is why I did the code below

Code: Select all

<?php 
$page = $_GET['page'];
$urlcheck = $page.".php";
if (!file_exists($urlcheck)) {
    include '404error.php'; // Changed this so that the file is included in the template instead of a whole page!
}
?> 
To my understanding this only checks for a file within the current DIR which would only contain the index file and all the other files to include. (Not The config file) and if not found it would include the 404 Error page instead.

Maybe This is a bad way of doing it and arrays is just has easy to do, but I do not want to add a file name to the array every time a page is created. Could you further explain how this is a security risk if I scan the "page=" before I do anything with it.?

any other suggestions for this without having to use a template system like smarty or something? :banghead:

Re: Need Some Advice

Posted: Mon Oct 18, 2010 10:50 am
by greyhoundcode
File_exists can be used with various wrappers, depending on your setup:
php.net wrote:As of PHP 5.0.0, this function can also be used with some URL wrappers. Refer to List of Supported Protocols/Wrappers for a listing of which wrappers support stat() family of functionality.
So if someone passes http://www.unknowncode.ru/script within $_GET['page'] then the PHP engine may well attempt to include the output from http://www.unknowncode.ru/script.php, if that is allowed within your particular setup.
php.net wrote:If "URL fopen wrappers" are enabled in PHP (which they are in the default configuration), you can specify the file to be included using a URL (via HTTP or other supported wrapper - see List of Supported Protocols/Wrappers for a list of protocols) instead of a local pathname. If the target server interprets the target file as PHP code, variables may be passed to the included file using a URL request string as used with HTTP GET. This is not strictly speaking the same thing as including the file and having it inherit the parent file's variable scope; the script is actually being run on the remote server and the result is then being included into the local script.

Re: Need Some Advice

Posted: Mon Oct 18, 2010 12:37 pm
by VladSun

Re: Need Some Advice

Posted: Mon Nov 15, 2010 10:16 am
by timWebUK
John Cartwright wrote:@timWebUK: A switch statement is essentially a whitelist already, so there is no use for having an array of accepted values, then have to repeat a case for those values. Then, you can use the default case to catch invalid values.
Valid point, I guess I'm just unnecessarily using processing power in my example.