Page 1 of 1
URGENT - PHP security with includes
Posted: Fri Apr 28, 2006 9:16 pm
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
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
Posted: Fri Apr 28, 2006 9:30 pm
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']);
Posted: Sat Apr 29, 2006 1:40 am
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.
Posted: Sat Apr 29, 2006 3:14 am
by Ollie Saunders
Personally I would never be in the situation where you have to write:
Putting a variable in an include statement in any way shape or form is unnecessarily risky.
Posted: Sat Apr 29, 2006 3:33 am
by AKA Panama Jack
ole wrote:Personally I would never be in the situation where you have to write:
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.
Posted: Sat Apr 29, 2006 3:39 am
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
Posted: Sat Apr 29, 2006 6:38 am
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)
Posted: Sat Apr 29, 2006 9:34 am
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?
Posted: Sat Apr 29, 2006 1:47 pm
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.
Posted: Sat Apr 29, 2006 3:25 pm
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.");
}
?>
Posted: Sat Apr 29, 2006 4:56 pm
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.