[lug] Safely Parsing PHP Parameters

[Zan Lynx]

On Wed, 2007-10-10 at 11:57 -0600, Bill Thoen wrote:
> On my web server, I've got a PHP script that displays text from various 
> essays by Thoreau and it accepts an essay number and a page number from 
> the URL. Since I know that evil people can do terrible things with a 
> poorly protected PHP script, I just wanted to check with you all to see 
> if I'm doing this safely. Essentially the PHP code that reads the 
> parameters looks like:
> 
> if (isset($_GET['essay'])) {
>    $essay = (int)$_GET['essay'];
> } else {
>    $essay=0;
> }
> if (isset($_GET['page'])) {
>    $page = (int)$_GET['page'];
> } else {
>    $page = 0;
> }
> 
> I then do checks to make sure the numbers are in the correct range, and 
> if so, I load the requested page from a directory that is outside the 
> web tree. I'm expecting URLs that look like:
> 
> http://thoreaufortheday.com?essay=3&page=1
> 
> but given my code for reading the parameters and my expectation of what 
> this script will encounter, are there any obvious security holes that I 
> need to consider?

A few comments:

Performance:
If the essay and page numbers never change their contents, I would
suggest replacing the PHP with static HTML pages.  Pregenerate them,
perhaps in a format like /essay/3/page/1.html.  If the idea is to pick
one per day then a top-level PHP script could slap it inside an IFRAME
or redirect to it.  

Then use mod_expire (I think) in Apache to set that directory tree to
cache for 10 years. (If you ever need to change the file contents in the
future, change the path).  You should do this for your image files also.
I noted with Firebug that the caching headers seem to be only the
default If-Mod-Since Apache does automatically for static files.

Or you could put an Expires header into the PHP output.

Imagine if 10 people behind a company or ISP web proxy all hit your
site.  With proper caching you get only 1 hit and save 9x bandwidth.

For PHP security, make sure your php.ini file does not enable
auto-variables from GET and POST parameters.  See:
http://www.php.net/manual/en/ini.core.php#ini.register-globals

And I think forcing the conversion to int will make your code safe
enough.  As long as no one can do a ?essay=../../../../../etc/passwd or
anything like it.
-- 
Zan Lynx <zlynx at acm.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.lug.boulder.co.us/pipermail/lug/attachments/20071010/daa9a30a/attachment.pgp>