Jump to content

How secure is my anti-directory transversal code?


nituvious

Recommended Posts

Hi guys, I wrote this speck of code to prevent directory transversal. However, I'm not that great with security issues, so I would like some of the gurus to offer pointers/tips/hints as to whether my code is safe or not and how to improve it.

		$pageID = $_GET["pageid"];
	$pageNewIDLower = strtolower($pageID);
	$pageNewID = ereg_replace("[^A-Za-z0-9]","",$pageNewIDLower);
	if (strstr($pageNewID,"../") || strstr($pageNewID,"%") != true) {
			// do stuff
	}
	else {
			include("pages/home.htm");
	}

If this looks wrong, let me know. I didn't take it directly from my php code as I'm on a cell phone at the moment.

Once you've removed all characters which are not alphanumeric, it's not possible for there to be any dot, slash or percent symbols left in $pageNewID.  So the strstr() tests are redundant.  I'm not sure if that regexp is doing what you want it to though.  For example, it'll do these conversions

 

../../../etc/passwd  =>  etcpasswd

files/file_5.txt => filesfile5txt

 

It'll remove file extensions as well as any legitimate directory names.  I think you need to allow dot and slash in the regexp, and keep the strstr tests.

 

Is this running on unix or windows?

Windows, but I would like to have a linux version as well.

The ereg_replace() is doing what I intended. In my code I check through a folder if a page exists with separate extensions like .php, .txt, .htm. There's not supposed to be any underscore within the page names, but I could change that. I was concerned that it would still be possible for someone to transverse directories because I use include(); function.

Ok I get it now.  No, there is no chance, because you have removed all characters except letters and numbers already.  That means that "../" has been removed by the regexp already.  The regxp is all you need to be secure.

 

You can add underscore to the regexp safely if needed.

Archived

This topic is now archived and is closed to further replies.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.