[RESOLVED] PHP includes and security issues

[surfgrafix]

I have a weather site and decided to do a major upgrade a couple years ago. Until that point I had not used PHP, so i am definitely a newbie. One section of it lets a user choose a year and a hurricane from drop down menus, it then reads a mysql database and plots the path of the hurricane based on lat/long and color coded based on intensity. Everything has been working fine until lately.

I think in my hurry to understand how to get PHP to do what I needed (what I thought was the complciated stuff - working with database and images), I missed some basic security stuff. I have searched you site and found one similar issue, but not the same. The site uses a index.php file which then includes a header, menu, and content. When the user clicks on a menu item, it loads index.php?content=menuitem.

Recently, looking at the logs, I found where someone was typing in a URL - index.php?content=http://website.com/file. Once they did this, additional php files were added to my folders. I deleted all the extra php files, denied the ip addresses using .htaccess, and contacted the hosting company. They were very helpful and found one file I had missed. I then immediately changed my password. I was fine for a few weeks, but they struck again today using a different IP address.

Additionally, there is a part of my site that uses an include from another server (it downloads several small text files from National Weather Service servers). So I would like to be able to keep that functionality.

Here is part of my index.php file...

<?php

include ("header.html");
print "<span class = \"item\"> \n";

include ($content);
print "</span> \n";

print "<span class = \"menuPanel\"> \n";
include ($menu);
print "</span> \n";

?>

Is there a way to somehow check $content (this one should always be a local file, not a URL)?

Thanks for any help!

[NogDog]

Never, ever trust data from external sources, including post, get, and cookie values.

If you expect $content to only be a file name, then use the [man]basename/man function to strip anything other than just the name.

Never, ever include()/require() remote files if you absolutely, positively do not have to; and ensure that when using any external data (form inputs, cookies, etc.) to specify a file for including/requiring, that you have thoroughly validated/sanitized it against any possibility of including a malicious script.

If you just want to use HTML output from a remote source, use [man]readfile/man instead, as it will not execute any PHP code embedded in the text it receives. Or you could use [man]file_get_contents/man or even [man]cURL[/man] functions to retrieve the text, then parse it, filter it, and sanitize it as desired (without ever executing it) before outputting it.

[surfgrafix]

Thanks for the quick response!

I have made the changes you suggested - using basename() since it should always be local files and other include() functions for text file from another server are now readfile() functions.