How secure is my anti-directory transversal code?

[nituvious]

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.

[btherl]

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?

[nituvious]

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.

[btherl]

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.