Is this include script secure?

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
mat106
Forum Newbie
Posts: 16
Joined: Wed Aug 31, 2005 2:52 am

Is this include script secure?

Post by mat106 »

Hello.

After reading the relevant threads about secure includes i've made some changes to mine - Could readers tell me whether the following include script (that includes a particular page of an article) and navigation script (that provides navigation for the article) are in any way vulnerable?

include script

Code: Select all

if (empty($_GET['page'])) @require_once ("1.htm");
			else
				{
				switch ($_GET['page']) {
				case "1": @require_once("1.htm"); break;
				case "2": @require_once("2.htm"); break;
				case "all":
				for ($i=1; $i<3; $i++)
					{
					@require_once ("$i.htm");
					}
				break;
				default: @require_once("1.htm"); break;
				}
			}
navigation script

Code: Select all

if (empty($_GET['page']))
				{
				echo "<li><a href='?page=all'>all</a></li>";
				echo "<li><a href='?page=2'>2</a></li>";
				echo "<li>1</li>";
				}
			elseif (!empty($_GET['page']) && is_numeric($_GET['page']) && $_GET['page'] < 3 && $_GET['page'] > 0)
				{
				echo "<li><a href='?page=all'>all</a></li>";
				for ($i=2; $i>0; $i--)
					{
					if ($i == $_GET['page'])
						{
						echo "<li>$i</li>\n";
						}
						else
						{
						echo "<li><a href='?page=$i'>$i</a></li>\n";
						}
					}
				$_GET['page'] = null;
				}
			elseif (!empty($_GET['page']) && !is_numeric($_GET['page']) && $_GET['page'] == "all")
				{
				echo "<li>all</li>";
				for ($i=2; $i>0; $i--) echo "<li><a href='?page=$i'>$i</a></li>\n";
				}
			elseif ($_GET['page'] < 0 || $_GET['page'] > 2 || $_GET['page'] !== "all")
				{
				echo "<li><a href='?page=all'>all</a></li>";
				echo "<li><a href='?page=2'>2</a></li>";
				echo "<li>1</li>";
				}
Thanks.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

Hi,

We had a similar thread not so long ago.

I prefer to use a similar method to the following:

Code: Select all

<?php

$pages = array('1', '2', '3', '4'); //add more if necessary..

@include_once($pages[$_GET['page']].'htm') or include_once('1.htm');

?>
You can also use names for pages instead of numbers, whilst keeping them indexed as numbers.. so you can pass a page ID instead of having to pass the actual name of the file/page :)
Post Reply