[Andora]

I am writing a little file navigation program.

I used scandir, then printed the directories, then printed the files.
I got this:
.
..
directory1
directory2
file1
file2
file3
file4
file5

The little dots at the top have me freaked out now. I clicked on the ".." one to see what would happen.

Since my file system is designed to let the user click and navigate around, the .. successfully took me up a directory level, and made some rather sensitive security/login information from my php scripts plainly visible.

The offending code is here...

if(isset($_GET['dir'])){$dir="links".$_GET['dir'];}else{$dir="links";}
$farray=scandir($dir);

I didn't think I'd need to escape the get variable, so obviously I am opening myself up there...

I have removed the "." and ".." links from appearing, and have made it so that only files with certain extensions will appear, but I am still nervous. There is nothing stopping anyone from putting \.. in the get variable themselves to find out about my file structure (idk if that matters).

Mostly, I am worried that there must be far more harmful things that can be put in there... I feel wide open for insertion, and I don't know what I can do in this case.

Any advice?

[CTphpnwb]

This should keep anyone from using higher level directories:

if(isset($_GET['dir']) &&  !substr('..', $_GET['dir'] ))
{

}

[Andora]

CTphpnwb, on 22 July 2010 - 02:25 PM, said:

This should keep anyone from using higher level directories:

if(isset($_GET['dir']) &&  !substr('..', $_GET['dir'] ))
{

}

clever use of substr! thanks

And then... This might be a paranoid question, but is that the only thing I need to worry about? ... Are there other commands that can be used there that I'm not expecting?

[CTphpnwb]

You might want to check your web server's settings. It might be possible for some one to see a folder by typing it's address without using a php file at all:

http://www.yoursite.com/somefolder/

If you don't want to, or can't modify Apache, you could put an index.php file in folders you want to protect. Just make the index.php file do a redirect to your home page.

[Motoma]

The traditional way of solving this problem is with file system permissions. If the user your web server is running under is not allowed to access the directories, you will never have to working about %2e%2e, .., or any other combination of characters, UTF-8 encoding, or other path traversal hacks returning you to directories.

On *nix, Apache typically runs as www, httpd, www-data, or something similar. Ensure that your root directory and all sub directories are set to o-w, and that only the web root is set to the www-data user.

On Windows, you will need to do a little more work.

If it's IIS, the user account will be something along the lines of IUSR_MACHINENAME, IIS_IUSR, or something along those lines. Ensure that this user has no access to any folders outside the web root.

If it's Apache, check what user the server is running as (Task Manager). Ensure this user cannot access any folders outside the web root. If it is running as SYSTEM or as your user account, CHANGE IT!

Security is hard! Difficult to implement, and nearly impossible to get right! It is better to rely on well established methodologies, best current practices, and operating system defined security mechanisms. This is no trivial task.

[Andora]

Hmm, sounds like that is beyond the scope of my issue, and something I'd just have to trust my host on...

My issue isn't that I don't want them to access the directory at all (the index.php is the mainpage), but that when they do it has to be via normal methods, not through my little file navigator, since it prints the raw contents of index.php, rather than viewing it after being interpreted by php...

For some reason, this code does not stop people from accessing a higher directory with &dir=/..

if(isset($_GET['dir'])&&!substr('..',$_GET['dir'])){$dir="links".$_GET['dir'];}else{$dir="links";}

nor have I had any luck with replacing it with strpos or strstr...
nor can I use str_replace('..'.''.$_GET['dir']) to remove them... wtf?

EDIT: nvm, looks like I wasn't uploading to the right server it might be working now, but very buggy... It's too late in the day for me to think clearly about this.

This post has been edited by Andora: 22 July 2010 - 10:56 PM

[CTphpnwb]

Hmm, I must have been tired. This works for me:

if(isset($_GET['dir']) && !strpos($_GET['dir'],'..'))
{
	$dir="links/".$_GET['dir'];
}else{
	$dir="links";
}

[Andora]

CTphpnwb, on 23 July 2010 - 03:28 AM, said:

Hmm, I must have been tired. This works for me:

if(isset($_GET['dir']) && !strpos($_GET['dir'],'..'))
{
	$dir="links/".$_GET['dir'];
}else{
	$dir="links";
}

There it is! That works perfectly, thank you.

[BGDeveloper]

I can suggest you another way with a simple array.
So you can set custom directories that can't be viewed for example.

$dir = $_GET['dir'];
$forbid = array(
'.','..','restricted', # the last one is some dir
);

if (isset($dir) && !in_array($dir, $forbid))
{
   // do some stuff
}
else
{
   // display error, or just simply redirect with header()
}

That's it.

[mahcuz]

CTphpnwb, on 23 July 2010 - 11:28 AM, said:

Hmm, I must have been tired. This works for me:

if(isset($_GET['dir']) && !strpos($_GET['dir'],'..'))
{
	$dir="links/".$_GET['dir'];
}else{
	$dir="links";
}

Just a reminder, if '..' is the first part of the string, strpos will return (int)0. Negating (!) a 0 leaves you will (bool)TRUE. Of course, this leads to problems, and you should therefore you strong-type-checking (===) in this situation.

strpos($_GET['dir'], '..') === FALSE

Mark.

[Motoma]

BGDeveloper, on 23 July 2010 - 04:52 PM, said:

I can suggest you another way with a simple array.
So you can set custom directories that can't be viewed for example.

$dir = $_GET['dir'];
$forbid = array(
'.','..','restricted', # the last one is some dir
);

if (isset($dir) && !in_array($dir, $forbid))
{
   // do some stuff
}
else
{
   // display error, or just simply redirect with header()
}

That's it.

BGDeveloper, what happens when someone puts in "restricted/../restricted"?

This is EXACTLY the reason why rolling your own security will only lead to being hacked.

[BGDeveloper]

This is just a simple example to begin with, I've never said that you're ready once you use the code, okay?
Believe me, if I want i could easily restrict you to access directories with my script.
Anyway, not hard feelings I'm not trying to prove anything, just suggesting..

[Andora]

mahcuz, on 24 July 2010 - 01:34 AM, said:

CTphpnwb, on 23 July 2010 - 11:28 AM, said:

Hmm, I must have been tired. This works for me:

if(isset($_GET['dir']) && !strpos($_GET['dir'],'..'))
{
	$dir="links/".$_GET['dir'];
}else{
	$dir="links";
}

Just a reminder, if '..' is the first part of the string, strpos will return (int)0. Negating (!) a 0 leaves you will (bool)TRUE. Of course, this leads to problems, and you should therefore you strong-type-checking (===) in this situation.

strpos($_GET['dir'], '..') === FALSE

Mark.

omg... thank you for pointing that out! However my confidence in this fix is declining sharply...

Quote

This is EXACTLY the reason why rolling your own security will only lead to being hacked.

Do I have an alternative? Please, do tell. I really hate how much this script opened me up, I really want to figure this out with whatever method works.

Quote

BGDeveloper, what happens when someone puts in "restricted/../restricted"?

Unless I'm terribly mistaken... strpos will detect the .. in the middle there, and it will fail?

[CTphpnwb]

Andora, on 24 July 2010 - 08:35 PM, said:

Unless I'm terribly mistaken... strpos will detect the .. in the middle there, and it will fail?

Yes, it will. It's the in_array that won't catch that, so you'd need to check using str_pos too.
Another possibility is to set up an array of "allowed" directories and check any path against it before performing scandir().