URGENT - PHP security with includes

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
michlcamp
Forum Commoner
Posts: 78
Joined: Mon Jul 18, 2005 11:06 pm

URGENT - PHP security with includes

Post by michlcamp »

I have been hacked several times this week by a Brazilian hacker/spammer who has used an include file link to exectute an external file which sends out thousands of spam emails

I see that I need to write some security into my scripts (a $pages array, I think) but don't know just how to get the job done...


to begin with, my non-secure link looks like this:

Code: Select all

http://mydomain.com/content.php?content=filename.php
and from what I'm reading, I need to set up an array of my text and .php files to make sure only files specified in the array will execute when configured in a link like the one above...

I find this code...

Code: Select all

$valid_pages = array(
	"apage.php"   => "",
	"another.php" => "",
	"more.php"    => "");

if (!isset($valid_pages[$page])) {
	// Abort the script
	die("Invalid request");
}
but don't know how to implement it or how to write the link after I have.

Unfortunately, like a lot of beginners, I've used the include

Code: Select all

<?Php Include("$content"); ?>
to display text files in a template-driven site, and now I obviously need to create some security around it.

specific questions are:
1) WHERE do I put the code for the $pages array,
2) How do I write the links to display '$content' in my template?

Any/all help much appreciated.
Thanks in advance.
mc
User avatar
Burrito
Spockulator
Posts: 4715
Joined: Wed Feb 04, 2004 8:15 pm
Location: Eden, Utah

Post by Burrito »

oops...I was wrong in that last post if you read it before I deleted it.

take a look at in_array()

ex:

Code: Select all

$valid = array("page1.php","page2.php");
if(in_array($_GET['content'], $valid))
   include($_GET['content']);
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

Post by AKA Panama Jack »

Your first mistake is passing the file names of php files you want to execute through a get or a post variable. If you need to specify different files to execute you should pass a reference name like...

Code: Select all

http://mydomain.com/content.php?content=js248fsk
Then have a reference array in the content.php...

Code: Select all

$valid_pages = array( 
	"js248fsk"   => "apage.php", 
	"wrwo352" => "another.php", 
	"25lt35t"    => "more.php");

if(isset($valid_pages[$_GET['content']]))
{
	include($valid_pages[$_GET['content']]);
}
else
{
	die("Invalid request");
}
Something like that would be better.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

Personally I would never be in the situation where you have to write:

Code: Select all

Include("$content");
Putting a variable in an include statement in any way shape or form is unnecessarily risky.
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

Post by AKA Panama Jack »

ole wrote:Personally I would never be in the situation where you have to write:

Code: Select all

Include("$content");
Putting a variable in an include statement in any way shape or form is unnecessarily risky.
Heh heh, nice sentiment but most applications do not have hardcoded include statements. They use variables in their include statements. Actually for most dynamic applications you would be hard pressed not to use variables in include and require statements.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

Heh heh, nice sentiment but most applications do not have hardcoded include statements. They use variables in their include statements. Actually for most dynamic applications you would be hard pressed not to use variables in include and require statements.
Well I've charged thousands of pounds for applications and none of them have ever used variables in include statements. This is possible because code itself is constant and when you need to include non-PHP stuff just use fopen
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post by timvw »

I've been in a situation where the variable would containt a relative path to the required page...
In order to make that they couldn't pull any tricks like ../../../passwd i've decided to compare the required $base_path to realpath($base_path . $request_path)
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

AKA Panama Jack wrote:Your first mistake is passing the file names of php files you want to execute through a get or a post variable. If you need to specify different files to execute you should pass a reference name like...

Code: Select all

http://mydomain.com/content.php?content=js248fsk
Then have a reference array in the content.php...

Code: Select all

$valid_pages = array( 
	"js248fsk"   => "apage.php", 
	"wrwo352" => "another.php", 
	"25lt35t"    => "more.php");

if(isset($valid_pages[$_GET['content']]))
{
	include($valid_pages[$_GET['content']]);
}
else
{
	die("Invalid request");
}
Something like that would be better.
Whats the difference between that and simply referencing the file? Other than making it an uglier URL, hard to bookmark, etc. Either way, the variable is being whitelisted, so why bother adding the obscurity?
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

Post by AKA Panama Jack »

Jcart wrote:Whats the difference between that and simply referencing the file? Other than making it an uglier URL, hard to bookmark, etc. Either way, the variable is being whitelisted, so why bother adding the obscurity?
By adding the obscurity they can't easily go fishing around for something that works. There are programs that can just start inserting program names into a url and run through them in the hopes of finding something that DOES get through. With this method you are virtually guaranteed that a program like that will never find anything.
d3ad1ysp0rk
Forum Donator
Posts: 1661
Joined: Mon Oct 20, 2003 8:31 pm
Location: Maine, USA

Post by d3ad1ysp0rk »

AKA Panama Jack wrote:
Jcart wrote:Whats the difference between that and simply referencing the file? Other than making it an uglier URL, hard to bookmark, etc. Either way, the variable is being whitelisted, so why bother adding the obscurity?
By adding the obscurity they can't easily go fishing around for something that works. There are programs that can just start inserting program names into a url and run through them in the hopes of finding something that DOES get through. With this method you are virtually guaranteed that a program like that will never find anything.

You're also guaranteed by using this:

Code: Select all

<?php
$goodFiles = array("home.php","about.php","contact.php");
if(in_array($_GET['page'], $goodFiles)){
  include($_GET['page']);
}
else {
  die("<b>Error:</b> Page not found.");
}
?>
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

Post by AKA Panama Jack »

I know but it is just bad form and practice to include the name of a PHP file in the get or post variable. It should be avoided at all costs. Atleast leave off the .php if you do anything.
Post Reply