Simple PHP Help

LiE · 6 Jan 2009 at 10:11

I'm trying to load content using PHP by using an include in my web page. I don't really know PHP but I've got this code which won't work.

PHP:

<?php	
$page = $_GET['page'];
		
if (!isset($page))	
{
$page = "home";
}
include('include/$page.txt');
	
?>

the urls look like:

PHP:

			<ul id="menu">
		    <li><a href="index.php?page=home">Home</a></li>
		    <li><a href="index.php?page=aboutme">About Me</a></li>
		    <li><a href="index.php?page=prices">Prices</a></li>
		    <li><a href="index.php?page=services">Services</a></li>
		    <li><a href="index.php?page=promotions">Promotions</a></li>
		    <li><a href="index.php?page=contact">Contact</a></li>
			</ul>

Here's the error: http://www.gemmahayden.com/dev/v2/index.php?page=home

To me it looks like it's not translating the variable $page? Any help would be great thanks.

pduk · 6 Jan 2009 at 10:21

Try using double quotes, which allow the parser to do variable expansion inside the string. i.e.

PHP:

include("include/$page.txt");

LiE · 6 Jan 2009 at 10:26

Thanks - works like a treat now. I knew it would be something simple

Inquisitor · 6 Jan 2009 at 12:07

A couple more points:

Firstly, isset will always return true in this case since $page is always set. What you want is file_exists.

Secondly, you should never use user input – anything that can be modified by the user – directly in file system (or database) operations without sanitising it first. Doing so allows an attacker to include any file on the server that the Apache process has access to by setting $page appropriately, which isn't a good thing.

See this security guide:

http://php.robm.me.uk/#toc-IncludingFiles

philjohn · 6 Jan 2009 at 12:19

msm722 said:
I'm trying to load content using PHP by using an include in my web page. I don't really know PHP but I've got this code which won't work.

PHP:

<?php $page = $_GET['page']; if (!isset($page)) { $page = "home"; } include('include/$page.txt'); ?>

the urls look like:

PHP:

<ul id="menu"> <li><a href="index.php?page=home">Home</a></li> <li><a href="index.php?page=aboutme">About Me</a></li> <li><a href="index.php?page=prices">Prices</a></li> <li><a href="index.php?page=services">Services</a></li> <li><a href="index.php?page=promotions">Promotions</a></li> <li><a href="index.php?page=contact">Contact</a></li> </ul>

Here's the error: http://www.gemmahayden.com/dev/v2/index.php?page=home

To me it looks like it's not translating the variable $page? Any help would be great thanks.

Please don't do this, what you have just shown is a directory traversal bug that would allow people to include any file on the server that is readable by apache.

Russinating · 6 Jan 2009 at 19:30

PHP:

$filename = "$a.php";
if (file_exists($filename)) {
include $filename;
} else {
include 'welcome.php';
}

..is what I use. Is this still susceptible to the above bug/hack/security flaw?

Ladforce · 6 Jan 2009 at 19:41

It could be, depending on where $a comes from

Russinating · 6 Jan 2009 at 22:20

Ladforce said:
It could be, depending on where $a comes from

Same as the OP's code, ie. domain.com/?a=x

Edit: Oh and I semi-forgot that this is included in the head of the page;

PHP:

$a = preg_replace('/\W/si', '', $_GET['a']);

I say forgot, but I got the include code from an old site that didn't have the protection above added to it yet, so at least that's reminded me!

suarve · 6 Jan 2009 at 22:42

msm722 said:
I'm trying to load content using PHP by using an include in my web page. I don't really know PHP but I've got this code which won't work.

PHP:

<?php $page = $_GET['page']; if (!isset($page)) { $page = "home"; } include('include/$page.txt'); ?>

the urls look like:

PHP:

<ul id="menu"> <li><a href="index.php?page=home">Home</a></li> <li><a href="index.php?page=aboutme">About Me</a></li> <li><a href="index.php?page=prices">Prices</a></li> <li><a href="index.php?page=services">Services</a></li> <li><a href="index.php?page=promotions">Promotions</a></li> <li><a href="index.php?page=contact">Contact</a></li> </ul>

Here's the error: http://www.gemmahayden.com/dev/v2/index.php?page=home

To me it looks like it's not translating the variable $page? Any help would be great thanks.

Shouldn't really do the infamous ?page=blahahah thing like that. You really need to use php's switch statement to check against a list of pages you have allowed.

LiE · 6 Jan 2009 at 23:07

I took this example from a website, as I'm not familiar with PHP. Could someone provide me with a simple fix please? Thanks.

Inquisitor · 7 Jan 2009 at 09:56

msm722 said:
I took this example from a website, as I'm not familiar with PHP. Could someone provide me with a simple fix please? Thanks.

As I said, look at Rob's security guide for PHP:

http://php.robm.me.uk/#toc-IncludingFiles

Though a better regex to use would be this:

PHP:

$page = preg_replace('/[\W\.-]/si', '', $_GET['page']);

This will disallow any file name that has characters other than letters, numbers, underscores, dots and hyphens in it by removing them.

LiE · 7 Jan 2009 at 10:18

Thanks I've added that in

LiE · 7 Jan 2009 at 14:01

This is the code I'm using now:

PHP:

		<?php    
		$page = preg_replace('/[\W\.-]/si', '', $_GET['page']);
		    
		if (!isset($page))    
		{
		$page = "home";
		}
		include("include/$page.txt");
		    
		?>

The problem I have now is $page isset and therefore home isn't allocated by default. Can anyone help me?

Ladforce · 7 Jan 2009 at 14:27

do the check on $_GET['page']. Also, before including the file, do a file_exists to make sure there is a real file there or the include will fail.

LiE · 7 Jan 2009 at 14:54

I don't know php Ladforce, is there any chance you could amend my code? Thanks

philjohn · 7 Jan 2009 at 15:25

msm722 said:
I don't know php Ladforce, is there any chance you could amend my code? Thanks

simply filter as others have posted and then do a

PHP:

<?
    if(!file_exists('files/' . $file))
        include('files/404.php');
    else
        include('files/' . $file);
?>

note that $file is the pre-filtered value.

LiE · 7 Jan 2009 at 15:41

ignore

LiE · 7 Jan 2009 at 15:52

This is what I have now. It sets the default correctly if I navigate to index.php.

PHP:

		<?php    
		$page = preg_replace('/[\W\.-]/si', '', $_GET['page']);
		    
		if (!isset($_GET['page']))    
		{
		$page = "home";
		}
		include("include/$page.txt");
		    
		?>

Added in the suggestion by philjohn but every page returned the 404.php as the include.

dbeckett · 7 Jan 2009 at 16:37

maybe try this(havent done php in ages)

PHP:

if (!isset($page))    //checks if a page has been entered
{
	$page = "home";//if not, set page to home
}
 
$page = preg_replace('/[\W\.-]/si', '', $_GET['page']); //take out all non alphabetical characters


if(!file_exists('files/' . $file))//check if page specified(or home if non given) exists
{
        include('files/404.php');//if it cant find the page include the error page
}
else
{
        include('files/' . $file);//if it can find the page, include it
}

dbeckett · 7 Jan 2009 at 16:41

Also are you editing all of this directly on your official website's webspace?

if so this aint a good idea, you'd be better setting up a personal webserver such as xampp and messing around with your code on your pc, where only you have access to it and if any problems arise no errors will appear to the public. Once you have completed your task you can then copy the new files to the public webspace