23.4.3. Security audit

Unfortunately, the above measures will not be enough to protect you, if the code is flawed. Thus, the most effective measure, is the one that needs a programmer and a security specialist together:

If you are a programmer, or if you hire one: inspect (audit) the code and "sanitize" the data, whenever it is input by the user. Sanitization is best achieved through a default-deny regular expression, like [^a-zA-Z0-9] - whatever matches the expression should be removed. The regular expression acts like a filter on user input. Make that filter as narrow as possible: if at all possible, allow only numbers, else letters and numbers. Make sure that any other characters, like symbols or punctuation, are converted to their HTML entity equivalents. Further, prefix and append a quote to all user input. For more programming tips, see Advanced SQL Injection In SQL Server Applications and Web Applications and SQL Injection.

If you rather prefer an explicit list of what to block, try this one, taken from SQL Injection Walkthrough:

Filter out characters like single quote, double quote, slash, backslash, semicolon, extended characters like NULL, carriage return, newline, etc, in all strings from:

• Input from users

• Parameters from URLs

• Values from cookies

For a numeric value, convert it to an integer before parsing it into an SQL statement.

Currently, PHP-Nuke checks with the following code for the presence of "bad code" in GET parameters:

foreach ($_GET as $secvalue) {
    if ((eregi("<[^>]*script*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*object*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*iframe*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*applet*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*meta*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*style*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*form*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*img*\"?[^>]*>", $secvalue)) ||
        (eregi("\([^>]*\"?[^)]*\)", $secvalue)) ||
        (eregi("\"", $secvalue))) {
        die ("I don't like you...");
    }
}

Notice that the expressions in the first argument of the eregi functions above are regular expressions (see Section 25.3). Their meaning is the following:

For the sake of example, we explain the first regular expression

<[^>]*script*\"?[^>]*>

meaning "anything that starts with "<" (that's the first "<" in the expression), immediately followed by zero or more occurrences (that's the "*" after the [^>]) of a character that is NOT (that's the "^" inside the "[^>]") ">" (that's what follows the "^" in the square brackets), immediately followed by "script", followed by zero or more occurrences of "t" (that's the "*" after "script", but it is wrong here, you probably should have a blank between "script" and "*", meaning zero or more blanks....but let's continue ), immediately followed by zero or one """ (that's the ""?" - don't let the double quotes mislead you, what we mean is "zero or one double quote" ), immediately followed by zero or more occurrences of any character that is not ">" (that's the "[^>]*" again...)". Phew...

As it stands, an expression like that will match everything that contains "< scriptxxxxxxxxx", where the x's stay for "whatever". This may be too restrictive for you, as you might want to allow words that contain "script" as part of the word, but not the word "script" itself. Try the following regular expression instead:

<[^>]*script +\"?[^>]*>

Uhmm...what's the difference? We have inserted a blank between "script" and the "*" immediately following it and have replaced the "*" with the "+". Thus, the "+" will refer to the preceding blank (meaning "one or more blanks", not "zero or more t's", as in the case of the original "script*"). Remember, the "*" and the "+" and the "?" all refer to the preceding "atom" - it is NOT like "dir *.exe" here!

Small, but crucial!

Further, go to modules/Forums/admin/common.php and have a look at the start of the code. There you can see how to use addslashes to "escape" potentially dangerous characters (like quotes). Since we are dealing only with GET parameters in this discussion, you need only the part that is relevant to the $HTTP_GET_VARS array:

//
// addslashes to vars if magic_quotes_gpc is off
// this is a security precaution to prevent someone
// trying to break out of a SQL statement.
//
if( !get_magic_quotes_gpc() )
{
        if( is_array($HTTP_GET_VARS) )
        {
                while( list($k, $v) = each($HTTP_GET_VARS) )
                {
                        if( is_array($HTTP_GET_VARS[$k]) )
                        {
                                while( list($k2, $v2) = each($HTTP_GET_VARS[$k]) )
                                {
                                        $HTTP_GET_VARS[$k][$k2] = addslashes($v2);
                                }
                                @reset($HTTP_GET_VARS[$k]);
                        }
                        else
                        {
                                $HTTP_GET_VARS[$k] = addslashes($v);
                        }
                }
                @reset($HTTP_GET_VARS);
        }
}

Chatserv has recently undergone PHP-Nuke a detailed scrutiny from the security point of view and came up with security patches for all 6.x and 7.x versions of PHP-Nuke that cover:

• New Abstraction layer conversion.

• Variables quoted on all sql queries.

• Security check added to most variables.

• Bugs in core files fixed.

• Previous sec-fix patches applied.

You should include Chatserv's patches in your security audit and try to stay current on the developments in this area as much as you can.