A variable can be a variable, did you know? Its something you may have learned in introductory PHP, like on p.32 of my copy of the Zend PHP Certification Study Guide. But while knowing data types is part of the job, its not always how you should code!

Here is an example of something I saw recently. Names have been changed to protect the innocent.

<?php
foreach ($fieldName as $field=>$type) {
$UserObject->setValueInDB($field, $$field);
}
?>

This is from a form submission script. There are a couple of transgressions I can think of, not least of all the reliance on the register_globals directive which is now off by default, and soon to be eliminated from a future release of PHP.

The variable variable part here is $$field, basically, what has been posted. The $fieldName value is a list of fields grabbed from the table, so you don’t trust $_POST. But what is the point in trusting the scalar equivalent of your posted value? You are getting farther away from certainty, not closer. A $_POST submission from an attacker could wipe out data because their $_POST array doesn’t have any keys that your table has. As well, if your $_POST array on your own page doesn’t have a $key=>$value that is also in $field=>type, well that value is going to get wiped out. In the case of a user profile edit page, a form page probably wont have all the fields that are posted. Especially if a developer doesn’t consider using table joins elsewhere.

One of the cardinal rules of programming is never trust user input. And I consider losing user data to be a deadly sin. But setting up a situation where you risk losing data in a field because of one additional field in the table for the user is downright dangerous.

One of the early-ish contributors to PHP, by virtue of being a C programmer, was no doubt familiar with the variable variable language construct, and appreciated its eloquence in CRUD scripts and elsewhere. You got your field names, so cycle through them in a looping construct and execute your value setting method.  He or she is forgiven for not realizing that eventually, with the blossoming of a thousand new web hosts and thousands more developers on the web, not only users had to be protected from themselves, but developers from themselves also. And so, as of release 4.2.0, register_globals was finally set to OFF by default. Many hosting companies have been slow to react, and even today set it to ON to support legacy software.

So while you have this spartan and eloquent structure, it relies on an obtuse language construct which in turn relies on data that is potentially not trustable. The solution to the above problem required a static array of field names that must not be overwritten. Of course, testing this once might reveal that data is being overwritten with empty values. Unintentionally. By design. Due to a deprecated directive and an obscure language construct.

Comments are closed.