PHP include security
in [General]
|
Prev: Include security?
Next: solution
From: Micky Hulse on 16 Apr 2010 00:57 Hi, Code: ========= ob_start(); switch ($this->command) { case 'include': @include($x); break; default: @readfile($x); } $data = ob_get_contents(); ob_end_clean(); ========= The above code snippet is used in a class which would allow developers (of a specific CMS) to include files without having to put php include tags on the template view. The include path will be using the server root path, and the include files will probably be stored above the web root. My question: What would be the best way to "clean" and secure the include string? Maybe something along these lines (untested): $invalidChars=array(".","\\","\"",";"); // things to remove. $include_file = strtok($include_file,'?'); // No need for query string. $include_file=str_replace($invalidChars,"",$include_file); What about checking to make sure the include path is root relative, vs. http://...? What do ya'll think? Any suggestions? Many thanks in advance! Cheers, Micky
From: Michiel Sikma on 17 Apr 2010 05:59 On 16 April 2010 06:57, Micky Hulse <mickyhulse.lists(a)gmail.com> wrote: > Hi, > > -snip- > > The above code snippet is used in a class which would allow developers > (of a specific CMS) to include files without having to put php include > tags on the template view. > > The include path will be using the server root path, and the include > files will probably be stored above the web root. > > My question: > > What would be the best way to "clean" and secure the include string? > > Maybe something along these lines (untested): > > $invalidChars=array(".","\\","\"",";"); // things to remove. > $include_file = strtok($include_file,'?'); // No need for query string. > $include_file=str_replace($invalidChars,"",$include_file); > > What about checking to make sure the include path is root relative, > vs. http://...? > > What do ya'll think? Any suggestions? > > Many thanks in advance! > > Cheers, > Micky > > -- > PHP General Mailing List (http://www.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php > > Hi, It depends. What's exactly do you want to prevent? It doesn't seem like a very big problem if someone tries to include an improper adderss or nonexistent file, since that would simply make $data an empty string (depending on your level of error reporting and whether you display or hide warnings). If the included file decides to call ob_get_clean() or something like that $data will be false. I can't think of what else you realistically want to prevent. Building a page with multiple templates is best done by using a good template class. Allowing the inclusion of external PHP files from a CMS will pose a risk if non-developers have access to the CMS as well. You're basically allowing anyone to add (potentially untested) code to a live site and I would recommend against doing it. If you want people to be able to include, say, additional HTML content, use file_get_contents() instead. Michiel
From: Micky Hulse on 17 Apr 2010 20:08 Hi Michiel! Thanks for the help, I really appreciate it. :) > It depends. What's exactly do you want to prevent? It doesn't seem like a > ...<snip>... > include, say, additional HTML content, use file_get_contents() instead. Very good points. My goal was to write a plugin that would allow me to include some static HTML template file and get the <?php include...?> tags out of my CMS template. With that said, I think the only people using this code will be the developers of the templates, and not your standard user. I opted to use output buffering and readfile() for the speed, and include() would be an option if developers want to execute the code in the included file. Would file_get_contents() be faster than readfile and output buffering? Would using file_get_conents() and eval() be faster than using include() and output buffering? Without boring you all to death, I am mostly interested in learning new stuff! I actually don't think anyone will use this code other than myself. :D But I definitely agree with all your points. Thanks so much for you help! Have a great day! Cheers, Micky
From: Micky Hulse on 17 Apr 2010 20:15 > What do ya'll think? Any suggestions? Sorry for the duplicate posting... I had some problems signing-up for the list. :( Also, I moved my test code to sniplr: <http://snipplr.com/view/32192/php-security-include-path-cleansing/> TIA! Cheers M
From: Michiel Sikma on 18 Apr 2010 13:23
On 18 April 2010 02:08, Micky Hulse <mickyhulse.lists(a)gmail.com> wrote: > Hi Michiel! Thanks for the help, I really appreciate it. :) > > > It depends. What's exactly do you want to prevent? It doesn't seem like a > > ...<snip>... > > include, say, additional HTML content, use file_get_contents() instead. > > Very good points. My goal was to write a plugin that would allow me to > include some static HTML template file and get the <?php include...?> > tags out of my CMS template. With that said, I think the only people > using this code will be the developers of the templates, and not your > standard user. > > I opted to use output buffering and readfile() for the speed, and > include() would be an option if developers want to execute the code in > the included file. > > Would file_get_contents() be faster than readfile and output > buffering? Would using file_get_conents() and eval() be faster than > using include() and output buffering? > I would prefer to use include() since it runs the code in the same context, and using both file_get_contents() and eval() is a bit of a detour. eval() also tends to be a lot slower than included code (though I'm not exactly sure how slow). I'm also not entirely sure whether file_get_contents() is slower than readfile(), but file_get_contents() is useful if you want to do something with your data rather than printing it right away. Michiel |