PHP Security Issues

[Freakish_05]

Following on from This Thread a mate of my mate has just been slagging me off because I've left a whacking great security hole in the includes!
Sure enough, he's right.

Homepage Oh Crap

As the site is database driven, I can't make a validation table within the code of index.php as links are added/updated/removed etc.
What I *think* I need is a bit of code that only opens pages from "/" as all the pages are in the same directory as index.php

Can anyone help?

 

[daz]

<?php

if(is_file("/home/USERNAME/public_html/" . $_GET['page']))
include($_GET['page']);

else { include("welcome.htm"); }


?>

Use something like that. Where /home/USERNAME/public_html/ is the path to your public html folder in your user directory.

 

[Berserker]

I'd take that one step further...

<?php

if(is_file("/home/USERNAME/public_html/" . $_GET['page'] . ".htm"))
include($_GET['page'] . ".htm");

else { include("welcome.htm"); }


?>

By hard-coding the extension you reduce the risk of other (non-HTML) files being picked up.

 

[Freakish_05]

I tried the above but I can still point the "page" variable to where ever I want

 

[Berserker]

Not possible if you've done it right. The is_file() test would fail.

 

[Freakish_05]

You guys were right all along.

It was me uploading the file to the wrong place lmao.

At any rate any time someone tries to alter the URL now they end up right back at the blog page

Thanks for your help. I can always count on the good people of OcUK forums

 

[robmiller]

Not quite done, heh.

http://www.unrealized-potential.com/index.php?page=index.php

 

[Dj_Jestar]

recursion woo!

you also need to check absolute paths. if someone enters '../../../somefile.blah' or similar, errors popup.

check with this:

<?php

$path = '/path/to/files';
$file = realpath($path . DIRECTORY_SEPARATOR . $_GET['page'] . 'php');

if ($file && strpos($file, $path) === 0 && is_readable($file)) include_once($file);
else include_once('default.page');

?>

 

[Freakish_05]

Aside from being incredibly annoying, who is going to care about the recursion?

Navbars were invented for a reason. The links work. I'm happy

 

[Dj_Jestar]

Your host will care. Lots.

 

[Freakish_05]

Ah, I see your point.

Will try implementing that code.

Edit: Nice one Dj_jester. Recursion no longer happens when going to URL http://www.unrealized-potential.com/index.php?index.php

 

[furnace]

Sorry dude but why have you done it like that anyway?

Why not have:

switch($_GET['page']){
default:
    /*what to do if page variable isn't specified*/
break;
case "gallery":
    include "gallery.php";
    /*or whatever else you wanna do if they want to see the gallery*/
break;
case "downloads":
    include "downloads/index.php";
    /*or whatever else you wanna do if they want to see the download bit*/
break;
case "favorites":
    include "favlinks.php";
    /*or whatever else you wanna do if they want to see the fav links bit*/
break;
case "blog":
    include "blog.php";
    /*or whatever else you wanna do if they want to see the blog bit*/
break;
}

Then you'd just say index.php?page=gallery&directory=unreal

Otherwise, I can go to:
http://www.unrealized-potential.com/gallary.php
And bring up some errors... it's better to have the gallery.php hidden by not letting anyone know it exists. As a matter of fact, it'd be even better if you called them gallery.inc.php and put them in an /include/ folder But that's just being picky lol

 

[Berserker]

Sorry dude but why have you done it like that anyway?

Why not have:

If you re-read the OP, you'll find out why not.

As a matter of fact, it'd be even better if you called them gallery.inc.php and put them in an /include/ folder But that's just being picky lol

Nope that's not picky - that's sound advice. You then stick a .htaccess on the include directory to deny direct access and you're done.

 

[Dj_Jestar]

or place everything above the document root, which is what is the current cool kids fad.

 

[furnace]

Nope that's not picky - that's sound advice. You then stick a .htaccess on the include directory to deny direct access and you're done.

Hehe I don't like sounding like an an*s Felt a bit bad saying that ontop of my first suggestion

Well he could have a table in the database to relate names to pages, rather then showing the name of the page, then lookup the name in the database to get the page.

But yeah it's too late to suggest how I'd do that... I need sleep

 

[Berserker]

or place everything above the document root, which is what is the current cool kids fad.

Oh yes, that works too. I do prefer to have everything in subdirectories below the parent script though - makes finding/moving stuff a whole lot easier.

I use the directories above the document root for backups and stuff like that.