Another day, another WTF

Can’t find the customer’s home country in the database? That’s ok; just pick any country with a vaguely similar-sounding name, that’s good enough.

A bizarre bit of code in the e-commerce software1 I’m currently fixing up does exactly that; it uses the Soundex algorithm to look for an approximate match to where the customer lives, according to the similarity of how the countries’ names are pronounced, rather than the more conventional considerations like geography.

There are 27 countries in the database that share a Soundex value with at least one other (mostly just pairs, but the largest matching group is 4: Ghana, Guam, Guinea and Guyana). In each group, all the countries would be rewritten to whichever was first alphabetically. Addresses in Greece would appear to be in Georgia; Norway became Nauru.

This sort of thing is nice and easy to fix (finding it is the hard part), but leaves a strange aftertaste… the insoluble mystery of just what was going on in the mind of whoever decided to write that code that made them think it would be a good idea…

  1. The software in question is an extended version of OSCommerce with lots of add-ons and customisation. I’m not sure whether the code in question originates from one of the add-ons, or is a specific customisation of this site done by their previous developer. In a way, I hope it’s the latter, so that other sites aren’t being affected… 

import_request_variables(): When will PHP stop being insecure by design?

Re Bugtraq post PHP import_request_variables() arbitrary variable overwrite.

This sort of thing really brings it home how the PHP core team still
don’t seem to really understand security… or would rather sacrifice it
in the name of backwards, very backwards, compatibility.

If you’re going to provide a function like import_request_variables()
to replace the blatantly-unsafe register_globals, how on earth can you
get it so badly wrong that it’s even more unsafe?? I mean, security 101
for a function like this would be:

  1. Don’t overwrite any existing global variables (come on!)
  2. Failing to specify a prefix should be at least an E_WARNING.
  3. Stop pandering to people who wrote or still use bad code originating back in PHP3 days, and aim towards getting rid of the whole concept of registering global variable symbols from user-supplied data. It was always a bad idea, it’s never going to stop being a bad idea, just drop it.

I also get terribly bored of seeing bugtraq reports of a “full path disclosure” bug in some app (like it’s a big deal too), only to find that it is, once again, PHP itself that’s at fault. Sure, the sysadmin on a production machine should set display_errors = Off, but if it’s left on why does PHP show the full path? If the purpose is to help the programmer developing code (who somehow doesn’t have access to the server errorlog), then the programmer doesn’t need the full path, they already know where the docroot is — so when showing errors from scripts in the docroot, why not only show the path relative to the docroot? And if from an included or required file outside the docroot, just the last directory component ought to suffice. The full path can still go in the error log, and then maybe developers would learn to use it instead of relying on errors going to screen…

PHP can be secure, but it really needs to stop offering features that are insecure by definition.