Posted by admin (Graham Ellis), 25 January 2003
It can be quick and short to write a PHP script to handle benign data, but in real life you need to add a lot of tests and checks to your code if it's to be robust.
Here are two scripts I wrote during last week's PHP course.
The first accesses a mysqld on a host computer called bhajee, and looks up information from a table called dist in the database called test. Records are returned where the code column equals a value entered by the user on a form.
Code:<head><title>Look up a place in an SQL table</title></head> <body bgcolor=white> <h1>We know where you are!</h1> <form>What does your postcode start with? <input name=pc></form> <hr> <?php if ($_GET[pc] == "") { print ("Your results will come here"); } else { $dbid = mysql_connect("bhajee","trainee","abc123"); mysql_select_db("test",$dbid); $query = 'SELECT * from dist WHERE code = "'. $_GET[pc]. '"'; print ("My query is <b>$query</b><br>"); $result = mysql_query($query,$dbid); while ($record = mysql_fetch_assoc($result)) { print ("$record [code] is $record[description]<br>"); } } ?> |
|
20 lines of code, and the only thing that's really new is the use of mysql_fetch_assoc - it allows us to call up columns by name rather thanby number and it means that our code won't need changing if an extra column is inserted. But lets's see the same code with a reasonable level of checks against users who enter invalid characters, broken networks, missing servers, etc. Like all good programs, I've commented it too ... and the code has risen to three times its original length.
Code:<head><title>Look up a place in an SQL table</title></head> <body bgcolor=white> <h1>We know where you are!</h1> <form>What does your postcode start with? <input name=pc></form> <hr> <?php
/////////////// See if the user completed a form or not if ($_GET[pc] == "") { print ("Your results will come here"); } else { /////////////// And if the user DID complete a form, make sure that the /////////////// data entered didn't contain any illegal characters $bpos=strspn( strtolower($_GET[pc]), "abcdefghihjlmnopqrstuvwxyz"); if ($bpos < strlen($_GET[pc])) { $pc = $_GET[pc]; ?> You are only allowed letters in the string to search in this particular example. You didn't :-( ....<br> <?php die ("First bad character was '$pc[$bpos]'"); }
//////////////// Connect to the MySQL server, and flag any errors if //////////////// the connection cannot be established $dbid = mysql_connect("bhajee","trainee","abc123"); $dbid or die ("Sorry - SQL server is unavailable");
//////////////// Select the database called "test" and flag any errors //////////////// if the database selection fails mysql_select_db("test",$dbid) or die("No database called test");
/////////////// Make up the query. Note - we have already checked the /////////////// incoming variable for illegal characters, so there's no /////////////// need to addslashes here, nor to htmlspecialchars $query /////////////// in the printed text $query = 'SELECT * from dist WHERE code LIKE "%'. $_GET[pc]. '%"'; print ("My query is <b>$query</b><br>");
////////////////// Run the query, and flag any error that are thrown up ////////////////// Could be a problem if user "trainee" doesn't have select_priv $result = mysql_query($query,$dbid) or die("Unable to run your query");
////////////////// Fetch results back. No errors will be flagged; a false ////////////////// return will come back when results are completed while ($record = mysql_fetch_assoc($result)) { ////////////////// Apply htmlspecialchars to the fields back from the database ////////////////// in case the have lessthan ampersand quote or greaterthans in them print (htmlspecialchars($record [code])." is ". htmlspecialchars($record[description])."<br>"); $counter++; }
////////////////// If the result set was empty, tell the user - don't just give ////////////////// back an empty window! if ($counter < 1) { print ("Nice input - pity it didn't match"); } } ?> |
|
Points for you to check for every PHP script like this that you right:
- Do all the database calls actually work? - don't assume that they do.
- Has the user entered anything "naughty" such as < or backquote in his input?
- Might there be quotes and other special chartacters to protect from MySQL?
- Might there be < and & characters to protect from HTML?
- Might there be a zero result set? If so TELL the user
- Might the user enter something he's searching for within a field? Consider using LIKE or RLIKE rather than =.
This page is a thread posted to the opentalk forum
at
www.opentalk.org.uk and
archived here for reference. To jump to the archive index please
follow
this link.