nituvious Posted October 24, 2010 Share Posted October 24, 2010 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. Link to comment https://forums.phpfreaks.com/topic/216722-how-secure-is-my-anti-directory-transversal-code/ Share on other sites More sharing options...
btherl Posted October 24, 2010 Share Posted October 24, 2010 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? Link to comment https://forums.phpfreaks.com/topic/216722-how-secure-is-my-anti-directory-transversal-code/#findComment-1125976 Share on other sites More sharing options...
nituvious Posted October 25, 2010 Author Share Posted October 25, 2010 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. Link to comment https://forums.phpfreaks.com/topic/216722-how-secure-is-my-anti-directory-transversal-code/#findComment-1126072 Share on other sites More sharing options...
btherl Posted October 26, 2010 Share Posted October 26, 2010 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. Link to comment https://forums.phpfreaks.com/topic/216722-how-secure-is-my-anti-directory-transversal-code/#findComment-1126466 Share on other sites More sharing options...
Recommended Posts
Archived
This topic is now archived and is closed to further replies.