Insecure code

yr8601

I use only a few lines of php on my homepage but it turned out that they are vulnerable to hack-attacks. Can anybody tell me what's wrong with them and how to fix the problem ?

In the "head" section I have
?php if ($main == "") { $main = "report.htm"; } ?

In the "body" section there is
?php if ($main == "$main") { include ("$main"); } elseif ($main == "")
{ include ("report.htm"); } else { include ("report.htm"); } ?

Any help will be highly appreciated. Thank-you,
Sebastian

Weedpacket

What is

if ($main == "$main")

supposed to do? All it does is convert $main to a string and then see if it is equal to $main (which, funnily enough, is pretty much always true - especially if $main is coming from a client request - which, incidentally, should be accessed with the $GET, $POST, or $_REQUEST arrays.)

And I don't see any checking to ensure that the page requested is one that the user is allowed to see.

planetsim

What im curious is this

include ("$main");

Make sure you use an array with files that exist on the server than check with $main if it exists if so then they can view it else they get something entirely different (e.g. a 404 Error).

yr8601

Thank you both very much for your comments. Since I don't know how to handle the array function here, I created a primitive construct and hope it makes sure that only the four named html-files can be shown.

<?php if ($main == "report.htm") { include ("report.htm"); }
elseif ($main == "projekte.htm") { include ("projekte.htm"); }
elseif ($main == "termine.htm") { include ("termine.htm"); }
else { include ("forscher.htm");} ?>

Is this hacker-proof ?

planetsim

try using a switch statement

the typical if/elseif/else statement should be this

if () {

} else if () {

} else {

}

No more, as PHP must check each result individually.

Here is a small example of using the array feature i said

 $pages = array(1 => 'Page01.php', 2 => 'Page02.php', 3 => 'Page03.php');
$default = 'Page01.php';
include in_array($_GET['go'], $pages) ? $pages[$_GET['go']] : $default;

yr8601

Once again: Thank-you very much. This time I tried to apply the switch statement because I didn't get the arrays working. Here's the result:

<?php switch ($main) {
case 'projekte.htm': include ("projekte.htm"); break;
case 'termine.htm': include ("termine.htm"); break;
case 'forscher.htm': include ("forscher.htm"); break;
default: include ("report.htm"); break;
}
?>

Is this a secure code ?

planetsim

Yes because the user cannot depending on your php.ini settings open a file from outside the www directory.

yr8601

Wonderful ! I never would have made it without you ! A big great Thank-you to the other side of the world, and all the best. Hope you're having a wonderful late summer or early autumn. Bye for now.