include security -what works, what dont?

[juglesh]

please comment on the following methods of preventing cross site
scripting and/or other nastiness:

1:
$pages = array('home','contact','about','links' );
// could also build this array with readdir('MySafeDir') ??
if( in_array($_GET['page'], $pages) )
{ include $_GET['page'].".php";}
else {die("Nice Try."); }

2:
include "my_prefix_".$_GET['page'].".php";

3:
include "my_safe_dir/".$_GET['page'].".php";

4:
include_path=".:/myIncludes";
include $_GET['page'].".php";

--
thanks for your time
juglesh

[J.O. Aho]

juglesh wrote:

please comment on the following methods of preventing cross site
scripting and/or other nastiness:

1:
$pages = array('home','contact','about','links' );
// could also build this array with readdir('MySafeDir') ??
if( in_array($_GET['page'], $pages) )
{ include $_GET['page'].".php";}
else {die("Nice Try."); }

2:
include "my_prefix_".$_GET['page'].".php";

3:
include "my_safe_dir/".$_GET['page'].".php";

4:
include_path=".:/myIncludes";
include $_GET['page'].".php";

You should see to remove all '..' from the paths, otherwise people could
navigate outside your secure directory.

The best IMHO is to use aliases for pages, and you hard code what the alias
mean, this way it will difficult to get the php script to display something
else than those pages you want.
//Aho

[juglesh]

J.O. Aho wrote:

juglesh wrote:

please comment on the following methods of preventing cross site
scripting and/or other nastiness:

1:
$pages = array('home','contact','about','links' );
// could also build this array with readdir('MySafeDir') ??
if( in_array($_GET['page'], $pages) )
{ include $_GET['page'].".php";}
else {die("Nice Try."); }

2:
include "my_prefix_".$_GET['page'].".php";

3:
include "my_safe_dir/".$_GET['page'].".php";

4:
include_path=".:/myIncludes";
include $_GET['page'].".php";

You should see to remove all '..' from the paths, otherwise people could
navigate outside your secure directory.

so, just replace .. with nothing? and that would apply to #3? so, if
they go '../passwords.txt or whatever, that would make my include be
equivalent to "my_safe_dir/up a directory to
root/".$_GET['page'].".php" and they can include something on the
root, or however many ../ they use?
The best IMHO is to use aliases for pages, and you hard code what the alias
mean, this way it will difficult to get the php script to display something
else than those pages you want.

I see, so,
if($_GET['page']=home){include myhomepage.php;}
if($_GET['page']=contact){include mycontactpage.php;} ??

[Gordon Burditt]

>> > 4:

> include_path=".:/myIncludes";
> include $_GET['page'].".php";
>
You should see to remove all '..' from the paths, otherwise people could
navigate outside your secure directory.

so, just replace .. with nothing? and that would apply to #3? so, if

My preference would be to return an error message to the user
(something similar to the FBI warning on video tapes) and
nothing else.
they go '../passwords.txt or whatever, that would make my include be
equivalent to "my_safe_dir/up a directory to
root/".$_GET['page'].".php" and they can include something on the
root, or however many ../ they use?

Yes.

I don't think I would want to let the user specify a file name, but
if they can, there's a few checks I would want to do:

- The file name (component) contains only acceptable characters,
which might be alpha, numeric, and maybe period, underscore, and
minus. NO slash, meaning all the files need to be in the same
directory.

- Check the component against a complete list of acceptable values
(no pattern-matching, a COMPLETE LIST of possible values, possibly
translating the value in the process).

For example, I occasionally have a page where you can select a sort
order with $_GET['order'], using a small set of named orders. The
names are supposed to make sense to the page maintainer and maybe
to a user reading the URL, but the user is really just supposed to
click a link with a longer description in the text, and not pay any
attention to the guts of the URL at all. I use a switch on
$_GET['order'] which sets a variable with the SQL order clause in
it. The name of the order has no necessary relationship to the SQL
fields involved (e.g. you might have order=date, order=datedesc and
order=name. The SQL fields involved might be signupdate, lastname,
and firstname).

The best IMHO is to use aliases for pages, and you hard code what the alias
mean, this way it will difficult to get the php script to display something
else than those pages you want.

I see, so,
if($_GET['page']=home){include myhomepage.php;}
if($_GET['page']=contact){include mycontactpage.php;} ??

That's the idea, but I think you are missing some quotes.

Gordon L. Burditt

[juglesh]

Gordon Burditt wrote:

> 4:
> include_path=".:/myIncludes";
> include $_GET['page'].".php";
>

You should see to remove all '..' from the paths, otherwise people could
navigate outside your secure directory.
so, just replace .. with nothing? and that would apply to #3? so, if

My preference would be to return an error message to the user

I've been hacking around, and I cant traverse(?) dirs with ../ if i use
a prefix, like "pre_".$GET['page'].".php" But, I'm not that
devious...

I want to guard against including remote xss nastiness, too. So, it
seems like the prefix thing works for that, they can try to include
'pre_http://badsite.com/badscript.php', but i dont see how thats going
to do anything. What if I str_replace('http',''), is there another way
around that?
- The file name (component) contains only acceptable characters,
which might be alpha, numeric, and maybe period, underscore, and
minus. NO slash, meaning all the files need to be in the same
directory.

yeah, for most of the easy page=home type pages, this works:
$_GET['page'] = ereg_replace("[^[:alnum:] ]","",$_GET['page']);

and then,
if (!is_file($_GET[page].".php"))
{header ("Location: http://www.domain.com/404.php"); die;}

The best IMHO is to use aliases for pages, and you hard code what the alias
mean, this way it will difficult to get the php script to display something
else than those pages you want.

I see, so,
if($_GET['page']=home){include myhomepage.php;}
if($_GET['page']=contact){include mycontactpage.php;} ??

That's the idea, but I think you are missing some quotes.

yup