New today is a post on Smoking toooooo much PHP entitled Six deadly PHP sins, this week…. The article gives a quick list of tips for PHP developers on why they definitely should avoid in their code.
One of the entries is titled Every variable should start somewhere. In a previous blog post, I mentioned how frustrating it was to be trying to read some code and asking yourself, ‘Where does this variable come from?!’. Alan labels PHP language constructs such as extract and eval as ‘evil’ because they disguise and obfuscate code. That isn’t news. However he makes a valid point – that using these types of shortcuts is not a security problem in itself – the security problem occurs when your code is too hard to understand and you inadvertently introduce additional problems.
Take register_globals for example. The register_globals feature in itself is not a security problem, but the fact that it makes understanding and reviewing code so much harder can easily lead to security holes throughout your code. The PHP manual states that ‘register_globals will inject (poison) your scripts will all sorts of variables’ which means that ‘writing insecure code is that much easier’.
When developing in PHP, the following steps can be taken to make your code easier to understand.
Don’t make your code overly complicated. If you have a choice between doing something on one line or doing it on four lines, choose the one that’s easiest to read and understand over the one that takes up less space. Take a similar approach to variable naming. A variable named $j is a lot more confusing than a variable named $paying_customers.
Break code into small, separate functions (or methods). When a piece of code is self-contained inside a function, it benefits from the local variable scope of that function. If that function is only 20 to 30 lines long (including comments), it can fit on your screen all at once, hence making it very easy to see where each variable is coming from. If you want to know where something was initialised, you won’t have to go off and grep for it (or give up and trust it) if its declaration/initlialisation all occurs within one page of where it is used. Of course, using the global keyword negates this benefit.
Comment your code. A common question from PHP newbies (or newbies to any language) is ‘how many comments should I use?’. There’s no right answer to this, and experienced coders who have spent some time maintaining other people’s code will probably know better than most. I would recommend that for every method or function, you make sure it’s clear what type of arguments are expected, what the return value if any means, and anything else that needs to be known about the method in order to use it. The purpose and return value should be clear from the name of the function/method, but to remove any doubt it doesn’t hurt to comment it. If the method should be ‘private’, you should indicate that. If the method should be called statically, you should indicate that. In addition to this, you should insert an inline comment every time you have done something somewhat complicated. Commenting styles are hotly debated topic, just as are coding styles (spaces or tabs?). The one thing that is certain is that you should comment your code. Commenting your code makes it easier to understand how it works, and thus easier to review it later and to ensure it is secure.
Unfortunately a lot of hard-to-understand code is, understandably, a result of laziness or unrealistic deadlines. Keep in mind that regardless of the current specification of the project, it is likely to change in future and when that happens, somebody is going to go through your mess of code and try to change its direction and purpose. If the code is self-explanatory, it will be a lot easier to modify it and doing so will be less likely to introduce new security problems.
Making sure your code is easy to read and understand is the first, and in my opinion the most important, step towards writing secure code.