[SOLVED] Is this secure?

[Altec]

I'm currently re-developing my personal site. Instead of using Wordpress, Joomla, or even a custom CMS, I'm using a really basic include() system. I'm using this code:

<?php

$pages = array('home','scripts','media','archives','contact','about');
$page = (in_array($_GET['page'],$pages) && !empty($_GET['page'])) ? trim(stripslashes(strip_tags($_GET['page']))) : 'naughty';
if($page != 'naughty') {
    require('./pagecontent/'.$page.'.php');
}

?>

After the correct page is required I then run grabmenu() and grabcontent() which are functions inside the included page file that output the menu and the content, respectively, for that page.

 

What I'm concerned about is the require function that includes the page file. I've heard to never use such a technique because hackers could potentially include files form their server. As it is, I don't see a hole that would cause that to happen. However, I'm wondering if someone more knowledgeable than me spots a problem.

 

EDIT: I should also mention I'm using this inside the included files:

if(!defined('SITE')) {
    die('You cannot access this content directly.');
}

[cunoodle2]

You are nice and secure there.  The ONLY options are...

 

('home','scripts','media','archives','contact','about')

 

There is nothing else to do there.  Just make sure you don't include the page anywhere else in the script and you will be fine.

[Alex]

There's no need for all that. If you've confirmed that it's already a valid value (in array), there's no need to strip anything from it.

 

$pages = array('home','scripts','media','archives','contact','about');
$page = (in_array($_GET['page'],$pages) ? $_GET['page'] : false;
if($page) {
    require('./pagecontent/'.$page.'.php');
}

[Philip]

To avoid a notice (undefined index) check to see if they even have the page in the url.

$pages = array('home','scripts','media','archives','contact','about');
$page = (isset($_GET['page']) && in_array($_GET['page'],$pages)) ? $_GET['page'] : 'home';
if($page) {
    require('./pagecontent/'.$page.'.php');
}

 

Also, I put in 'home' as the default if it isn't in the array or not selected, but you could always change that to another page.

[Altec]

To avoid a notice (undefined index) check to see if they even have the page in the url.

$pages = array('home','scripts','media','archives','contact','about');
$page = (isset($_GET['page']) && in_array($_GET['page'],$pages)) ? $_GET['page'] : 'home';
if($page) {
    require('./pagecontent/'.$page.'.php');
}

 

Also, I put in 'home' as the default if it isn't in the array or not selected, but you could always change that to another page.

 

Using this the if() check isn't even necessary, correct?

[Philip]

No its not needed, unless you wanted to do something different like show an error message. If you just want it to load a default page the code above will work fine.

[MadTechie]

correct

if($page) { 

is not needed