Page 1 of 1

Template security : To switch or not to switch?

Posted: Sun Dec 05, 2004 11:38 pm
by mhulse
TITLE: Template security : To switch or not to switch?

Hi fellow PHP'ers!

I am using a switch/case statement on my main index/template page for tighter security (code snippet below)...

What would be a better alternative to the below switch statement? Can I write something more compact? I would like to avoid having to add every page to the statement... anyone out there have better techniques/alternatives? I would love to know of a more modular/compact way that also keeps security tight/tighter... I am planning on writing a photo gal that targets multiple variables on main template page (a possible example: somePage.php?pageHolder1=somePage&pageHolder2=anotherPage&pageHolder3=someImage, and using below code, I would have to add all if I want to get it to work... Ugh...:

Code: Select all

<?php
		
		if (!isset($_GET["display"])) {
			$_GET["display"] = "home";
		}
		
			# Include the menu:
			include($_SERVER['DOCUMENT_ROOT'].'/folio/inc/nav.inc.php');
			
			# Figure out which page contents to display:
			switch($_GET["display"]) {

					// Home:
					case "home":
						include($_SERVER['DOCUMENT_ROOT']."/folio/gutz/home_gutz.php");
						break;
			
					// About:
					case "about":
						include($_SERVER['DOCUMENT_ROOT']."/folio/gutz/about_gutz.php");
						break;
						
					// Work:
					case "work":
						include($_SERVER['DOCUMENT_ROOT']."/folio/gutz/work_gutz.php");
						break;
					
					// Page default (404):
					default:
						include($_SERVER['DOCUMENT_ROOT']."/folio/gutz/error_gutz.php");
						break;
			
			} // End switch.
			
		?>
Hopefully I am making sense... please help. :)

Thanks a billion peeps, I appreciate any help you could send my way.

Cheers
Micky

Posted: Sun Dec 05, 2004 11:47 pm
by Steveo31
You could do like a little loop deal:

Code: Select all

<?php

switch($_GET['page']){
    foreach($_GET['page'] as $to_inc){
         case $to_inc:
              include 'path_to_file/'.$to_inc.'.php';
         break;
    }
}

?>
So if the URL was index.php?page=home then it would include home.php, but it may be a poss security risk... up to you.

Posted: Sun Dec 05, 2004 11:47 pm
by Kedo
Hi
You can try the following Code:

Code: Select all

if(file_exists($_SERVER['DOCUMENT_ROOT']."/folio/gutz/".$_GET["display"].".php")
   {
      include ($_SERVER['DOCUMENT_ROOT']."/folio/gutz/".$_GET["display"].".php");
   }
else
   {
      include ($_SERVER['DOCUMENT_ROOT']."/folio/gutz/error_gutz.php");
   }
Greetings Kedo

// Edit
I seem to be too slow ;)

Posted: Sun Dec 05, 2004 11:53 pm
by mhulse
Ooooh, nice guys, I likey! Great suggestions... good ideas :D

Testing things now. I love the forums!

I would love to see/read others suggestions... Thanks for the great suggestions Steveo31
and Kedo, you guys rock!

Posted: Mon Dec 06, 2004 12:02 am
by scorphus
Very nice first post, Kedo. Most first posts here are questions! Welcome to DevNetwork Forums!

Well, here is my suggestion:

Code: Select all

<?php

        if (!isset($_GET['display'])) {
            $_GET['display'] = 'home';
        }

            # Include the menu:
            include($_SERVER['DOCUMENT_ROOT'].'/folio/inc/nav.inc.php');

            $pages = array(
	            $_GET['display'] => 'error',
	            'home' => 'home',
	            'about' => 'about',
	            'work' => 'work'
            );
            $page = $_SERVER['DOCUMENT_ROOT'] . '/folio/gutz/' . $pages[$_GET['display']] . '_gutz.php';
            include $page;
?>
-- Scorphus

Posted: Mon Dec 06, 2004 2:49 am
by rehfeld
i would HIGHLY suggest ONLY including pages which you specifcally define as ok

you must anticipate what would happen if a malacious user were to use a url like

index.php?display=../../index
index.php?display=../../secret_dir/config

take a look at basename() and dirname(), they can help you avoid this

a pretty solid method would be as follows

Code: Select all

<?php


$avail_pages = array(
    'page1',
    'page2',
    'homepage',
    'path/to/foo',
);


$module = 'homepage.php'; // default

if (isSet($_GET['page'])) {
    if (in_array($_GET['page'], $avail_pages) && is_readable($_GET['page'] . '.php')) {
        $module = $_GET['page'] . '.php';
    }
}

include ($module);


?>

theres lots of ways to achive this type of functionality for your website, but generally, you want to keep the possible files to be included restricted to a very strict list or set of security checks.

Posted: Mon Dec 06, 2004 3:03 am
by kettle_drum
And as the site gets bigger you may consider using a database to hold the valid pages.

Posted: Tue Dec 07, 2004 2:10 pm
by scorphus
Oh... I thought my code could speak for itself but it seems not true though.

There aint need for doing anything else than what I showed above. No if conditions, no test for readable file, no need to check if item is in array, no foreach, etc. Let me introduce a more instructive and generally applicable perspective of the same idea:

Code: Select all

&lt;pre&gt;&lt;?php
// Copy the data from the URL's query string or set default value:
$_get_display = isset($_GET&#1111;'display']) ? $_GET&#1111;'display'] : 'home';
// All the following info could be stored in a database as kettle_drum
// courteously points out. Define a set of valid pages:
$valid_pages = array(
	'home' =&gt; 'index',
	'about' =&gt; 'the_company/index',
	'work' =&gt; 'workforus',
	'news/tech' =&gt; 'news/tech',
	'news/science' =&gt; 'news/sci',
	'news/politics' =&gt; 'under_construction',
	'contact' =&gt; 'contact'
);
// Here is the key! Two arrays get merged, one containing the value 'error'
// for the $_get_display index ($_GET&#1111;'display'] in most cases) and the other
// is the set of valid pages as defined above. When they get merged, if by
// any case $_get_display indexes a value in $valid_pages (i.e. if exists
// $valid_pages&#1111;$_get_display]) the value 'error' is replaced by the value in
// $valid_pages indexed by $_get_display (i.e. $valid_pages&#1111;$_get_display]):
$pages = array_merge(array($_get_display =&gt; 'error'), $valid_pages);
// In short $pages&#1111;$_get_display] can assume only 1 of 2 possible values:
// 1) 'error'
// 2) $valid_pages&#1111;$_get_display]
// Let's take a look at $pages:
print_r($pages);
// Now it is sure this is either a valid page or the error page:
$page = $pages&#1111;$_get_display] . '.php';
echo $page;
?&gt;&lt;/pre&gt;
Let's test it! For a normal access, say 'query-string.php?display=news/science', here is the output:

Code: Select all

Array
(
    &#1111;news/science] =&gt; news/sci
    &#1111;home] =&gt; index
    &#1111;about] =&gt; the_company/index
    &#1111;work] =&gt; workforus
    &#1111;news/tech] =&gt; news/tech
    &#1111;news/politics] =&gt; under_construction
    &#1111;contact] =&gt; contact
)
news/sci.php
Now imagine one malicious user trying to hack the application. When she tries 'query-string.php?display=../include/class.dbconnect', this is what she gets:

Code: Select all

Array
(
    &#1111;../include/class.dbconnect] =&gt; error
    &#1111;home] =&gt; index
    &#1111;about] =&gt; the_company/index
    &#1111;work] =&gt; workforus
    &#1111;news/tech] =&gt; news/tech
    &#1111;news/science] =&gt; news/sci
    &#1111;news/politics] =&gt; under_construction
    &#1111;contact] =&gt; contact
)
error.php
As it becomes clear, only pages specifically defined as valid will get included. Even an attempt to crack the app with 'query-string.php?display=+_(-)*$%@!/\|%13%10?'[]"' will fail:

Code: Select all

Array
(
    &#1111; _(-)*$%@!/\\|?''&#1111;]"] =&gt; error
    &#1111;home] =&gt; index
    &#1111;about] =&gt; the_company/index
    &#1111;work] =&gt; workforus
    &#1111;news/tech] =&gt; news/tech
    &#1111;news/science] =&gt; news/sci
    &#1111;news/politics] =&gt; under_construction
    &#1111;contact] =&gt; contact
)
error.php
I hope it will be useful. Soon I will post it in the Code Snippets forum, with bits of improvement.

Regards,
Scorphus.