Page 1 of 1

any sugestions ?

Posted: Tue Jan 03, 2006 9:57 am
by spamyboy
Well I'm creating counter-strike scripting website, with many scripts and etc. I dont use any php engines (php nuke or what ever).
Now to display scripts I use sessions

Code: Select all

<?
    $page=$_GET['page'];

    $id = array(
    '' => 'error.php',
    'ak' => 'downloads.php',
    'colt' => 'news.php',
	'brust' => 'about.php',
    );

    if (!isset ($id[$page])){
    echo "";
    }
    else {
    @include ($id[$page]);
    }
    ?>
But its wery discomfortable. So is there any sugestions, what culd I use ?

Posted: Tue Jan 03, 2006 11:05 am
by TS_Death_Angel
...sessions? I don't see them in your script.

Posted: Tue Jan 03, 2006 11:07 am
by spamyboy
sorry.. my mistake no sesions i n this script :) but jave you got any sugestions ? (I use sessions in login :))

Posted: Tue Jan 03, 2006 11:12 am
by Maugrim_The_Reaper
Any specific area you need suggestions on because the above script looks almost...well...normal. I'd do it the exact same way for any small site I built. Since you feed the user input into a conditional that matches values there's not even a real security risk I could moan to you about ;).

Only picky thing I'd point out is that using @include() over include() will supress any errors from the included file. I'd suggest removing that (making testing easier) and if you want to avoid displaying errors set display_errors to 0 via ini_set().

Posted: Tue Jan 03, 2006 11:15 am
by TS_Death_Angel
Hmm suggestions :P Well it's kind of hard to do, since it's your project and you do it how you want. :) But for my website, I have the GET variable ID read what page to load. For example:
http://~gone~/z64/logo.php?id=main loads main.php in my pages dir. I don't have all my pages prewritten within the script, however. They are loaded dynamically so that it's easier to add pages later. As for the sessions... what else do you need them for other than logging in?

Posted: Tue Jan 03, 2006 11:36 am
by Maugrim_The_Reaper
TBH I prefer the original system - the problem with not specifying the includes within the file is that user data could be manipulated.

So say you have a simple script:

Code: Select all

$page = $_GET['id'];

include($page . '.php');
I can now pass in almost any value, including ?id=../../../usr/data/secretfile or anything else and include any file on your current system. For this reason matching an ID to a pre-defined page is safer - the user data either has a valid ID or its simply not used, or provokes an error.

So although your suggestion is more flexible - its more open to attack by bad people on the internet...

Of course if you filter and validate your dynamic ID, its still workable - it's not a bad system, you just need to be aware of the security implications of user input before you blindly trust it.

For example, assuming your ID must follow a very simple rule, e.g. it can only be alphanumeric (A-Za-z0-9) you can safely use:

Code: Select all

$page = $_GET['id'];

if(!ctype_alnum($page)) // check whether the ID contains only letters and numbers
{
    trigger_error('Error: Invalid Page ID passed as URL parameter. This may have been a hacking attempt!', E_USER_ERROR); // create a custom error message and exit the script (E_USER_ERROR is a custom fatal error)
}

require('/path/to/mysite/' . $page . '.php');
You probably get the point by now I think - always filter that user input (users can change the ID in the url) to be certain its valid.

At this point even if the alphanumeric ID is incorrect - the use of require() instead of include() will create an error if the file does not exist.

You now have a safer script, producing errors, and not letting any user look for any other file on your system other than the directory you specify.



Concluding this long post - the first method that opened this thread is safer, the one you suggest is more flexible but only if correctly filtered for user input. So if you are making use of your method - please ensure you have the suggested filtering in place...

Posted: Tue Jan 03, 2006 11:46 am
by spamyboy
So idont neeed to change anything in your writen code ? (it looks safe to me :))

Posted: Tue Jan 03, 2006 11:49 am
by TS_Death_Angel
As I told you in PM, the includes are all prefixed with 'articles/' which is where my articles are stored, and none of the stuff in there is private so it shouldn't matter :P

Posted: Tue Jan 03, 2006 11:51 am
by spamyboy
For e.g. if i have things for sale in my web, maybe the safe way would be to keap them in other host ?