Karma's Nucleus DevBlog

Remember the What's wrong with this code? post from a week ago?

Niels catched most of the problems that I injected in the code. Allow me to describe the problems a little more detailed:

short_open_tag

When the short_open_tag is set to Off, the example code won't run at all! Instead, the client will see the PHP code, including the embedded MySQL password (if one is explicitly specified, that is). It's a good habit to always use <?php ... ?> and <?php echo '...' ?> instead of <? ... ?> and <?= '...' ?>

register_globals

First of all, this script relies on the register_globals setting to be set to On. With this setting, form values like the name in the code sample are made available directly through $name. While this makes writing code easy, it's a known cause for many security problems. You should make it so your code never relies on register_globals, but also that no unitialized variables are used in your code, since these could get filled out maliciously when register_globals is on.

In the sample code, the evil is done in the showGreeting function. The global $user line of code should be replaced by $user = $HTTP_POST_VARS['user']. (but even this will cause a problem, see the section about $HTTP_POST_VARS)

Even more problems might arise when relying on register_globals. There's another setting called variables_order. While I've never seen customizations of this one, it defines which values take precedence if e.g. "name" is defined as a form value as well as a cookie. By default, the value from the cookie will override the value from the form.

HTTP_POST_VARS

Many PHP coders have grown up to use the global $HTTP_POST_VARS variable to get form values out of the request. This even works in PHP5. Unless... the register_long_arrays setting is turned off.

The solution is to use the superglobal $_POST variable. This variable (along with it's friends like $_GET and $_COOKIE) have been introduced in PHP 4.1.0 to replace the (now obsolete) $HTTP_xxxx_VARS type globals.

One problem remains: how to make the code work on versions <4.1.0 as well? The approach used by Nucleus is to define a set of helper functions (getVar, postVar, ...) that use the correct variables depending on the PHP version Nucleus is running on. These helper functions solve some extra problems, as you'll see below.

magic_quotes_gpc

One lesser known setting: when magic_quotes_gpc is set to On, all incoming form data will be automatically escaped (gpc stands for "get", "post" and "cookie"). e.g. when you fill out "It's karma", the actual value received by the script will be "It\'s karma" (note the extra slash!). While this might be good for scripts that write incoming data directly to the database, it's generally undesired behavior. In our case, the code will malfunction and the special "Hi Karma!" message will not be displayed.

In come the Nucleus helper functions again. These functions will undo the magic that PHP added, if any. Below is an example of how the postVar funtion looks like for v4.1.0+

function postVar($name) {
  return undoMagic($_POST[$name]);
}

// removes magic quotes if that option is enabled
function undoMagic($data) {
  return get_magic_quotes_gpc() ? stripslashes_array($data) : $data;
}

Error reporting

One other thing you should be aware of, is that quite some error spitting will be done when the error_reporting setting is set to e.g. E_ALL. In our case, you'll get an error "Notice: Undefined index: name in filename.php on line 2" when you try to run the script.

Therefor, it's generally a good idea to explicitly specify the error logging in your code. For a debug build of Nucleus, the following code enables all errors except for notices:

error_reporting(E_ALL & ~E_NOTICE);

magic_quotes_runtime

And it doesn't end here: there's yet another little known setting that can cause problems when it is turned on: magic_quotes_runtime works much like magic_quotes_gpc, but operates only on data received from external sources (database queries, input files, ...). In our case, if the greeting in the database would be "It's nice to see you again!", the $greeting variable would end up with a value "It\'s nice to see you again!". Not exactly what we expected to happen.

To make sure this setting doesn't break your scripts, you'll need to add the following line of code at the start of your script:

set_magic_quotes_runtime(0);

Invalid XHTML

Quite obvious with this sample: it doesn't generate valid XHTML. It generates a blurb of HTML 4, which most browsers are forgiving enough to parse.

Main reason for including this error was to keep the example code compact.

ASP style tags:

Through the asp_tags setting, it is possible to use <% php code %> next to <? php code ?>. In this case, our script will break because of the following line:

// each <%user%> will be replaced by the user name

...which PHP will interprete as:

// each <?user?> will be replaced by the user name

Conclusion: never use <% ... %> inside comments.

SQL injection

Ok, what if ' or ''=' was entered into the form field? Well, you'd end up with the greeting message for the first row in the database. Further messing around with the form field value could make it possible to read arbitrary data out of the database. Passwords or access tokens, for example.

The key solution here is to always protect the data which you're using in SQL queries: the data should never be able to alter the query itself. Therefor, you should use the addslashes() function whenever sending strings to SQL queries. Also, if you've got a variable which is supposed to be an integer, it's always wise to wrap it into an intval call.

HTML injection

Ok, what if <body onload="alert('foo')"> was entered in the form field? Indeed: a messagebox is displayed. The problem here is that the input data isn't checked for malicious input at all. HTML code, JavaScript or CSS code can make it into the HTML code of the response page. This is how XSS (cross site scripting) issues are born.

Solution is to sanitize the input, or to make sure the right transformations are applied to the data before sending it to the output. In our case, we should htmlspecialchars on $user before writing it to the output stream.

Things that aren't errors but should be avoided nevertheless

The example code has uses different casing for function names: while the showGreeting and showForm functions are defined using camelCase, while the calls to those functions are made in all lowercase.

While this works perfectly in all PHP versions I know of, it makes your code look less organized. Having some consistency in code is good.

Errors that are missing

This list of common errors and problems is far from exhaustive. Other errors that could occur could include:

• Defining user-functions with the same name as a PHP-function which is introduced in a later version, or with the name of a currently not-loaded module. Nucleus used to have this problem with the xmlrpc_encode function, which is both defined in a PHP XML-RPC extension and in the XML-RPC library used by Nucleus. Damn that global namespace!