About Include()

[crays]

Hi, i'm trying to do this. If someone go to a link with a _GET value. like buy.php?item=bowl it will include the bowl.php page in it. My script is as below:


$taller=$_GET["item"];

if($taller = 'bowl') {
include("buy/bowl.php");
}
elseif($taller = 'clock'){
include("buy/clock.php");
}
elseif($taller = 'stick'){
include("buy/stick.php");
}
else {
header('Location:member.php');
}



i wonder what is wrong here? Because as i insert buy.php?item=bowl it will show me bowl.php but if i insert buy.php?item=clock it will also show me bowl.php. Same goes for buy.php?item=stick. Any help?

[GJay]

you're using '=' rather than '=='. The former is assignment which will evaluate to true, while you want equality '=='.

http://www.wellho.net/mouth/406_Assignment-equality-and-identity-in-PHP.html
explains it maybe a bit better

[crays]

Thanks mate, much more understandable.

[kbluhm]

Also have a look at switch():

$item = isset( $_GET['item'] ) ? $_GET['item'] : NULL;

switch ( $item )
{
case 'bowl':
include( 'buy/bowl.php' );
break;
case 'clock':
include( 'buy/clock.php' );
break;
case 'stick':
include( 'buy/stick.php' );
break;
default:
header( 'Location: member.php' );
}


Or, it appears the file names are identical to the $_GET['item'] value. You could use a file_exists() check and be able to add more files without having to change your control code each time:

$item = isset( $_GET['item'] ) ? $_GET['item'] : '';
$file = './buy/' . $item . '.php';

if ( file_exists( $file ) )
{
include $file;
}
else
{
header( 'Location: member.php' );
}

[masterofollies]

Dragon's Kingdom script?

[crays]

ah pretty nice. That's better. Thanks alot~

[kbluhm]

I realize this is a bit of an aged topic, but I was poking around the User CP and noticed a bad rep left on this message.
Horribly insecure, bad advice
The code in question is not at all insecure, and if someone has a warning like that I feel it would benefit the community to openly comment on it so we can all review your thoughts and opinions.

I think the person quickly scanned the code and was foolish enough to find it insecure without noticing that the file's location is specifically hard-coded to a path and extension in the $file variable before being checked with file_exists().


Or, it appears the file names are identical to the $_GET['item'] value. You could use a file_exists() check and be able to add more files without having to change your control code each time:

$item = isset( $_GET['item'] ) ? $_GET['item'] : '';
$file = './buy/' . $item . '.php';

if ( file_exists( $file ) )
{
include $file;
}
else
{
header( 'Location: member.php' );
}

[PeaTearGriffin]

Just wondering if there is any difference using:
$file = './buy/' . $item . '.php';
vs.
$file = 'buy/' . $item . '.php';

[anarchy3200]

I've not got a test machine at hand to check whether the extension could be circumvented but as for the location you can simply use ../ to change up a directory.

Using this as a hole would be more trial and error but by including arbitrary files from higher levels it could be possible to overwrite variables that could cause knock on effects. This is all speculation though.

In itself this isn't necessarily a security issue but as in this instance it was a finite list of addresses its more ideal to be safe than leave a possible hole.

[kbluhm]

Well consider my foot stuck firmly in my mouth. I'll just have to accept the fact that the quite obvious possibility of using '../..' completely slipped through the cracks on this one. For whatever reason I had it in my head that those were only parsed via realpath(), but that will simply modify the path to it's full representation.

It should also be noted that anyone copying and pasting quickly-written examples deserve the possibility of security wholes, but that's just me playing a bit of CYA. ;)