3

I've inherited a large and extremely buggy PHP codebase. It needs a thorough review and cleanup, but I'm in need of a stopgap fix in the meantime. In the long run it needs proper user input sanitisation, e.g. as per What's the best method for sanitizing user input with PHP?. However, this just isn't practical in the short term. It's a codebase of thousands of files comprising hundreds of thousands of lines of code and I need some way of blocking the majority of errors as soon as possible.

Many of the scripts use $_GET['id'] as an unsanitised parameter for database lookups. Is there any way of getting PHP to automatically preprocess the $_GET array along these lines:

if( isset( $_GET['id'] ) )
    $_GET['id'] = (int) $_GET['id'];

I'm well aware that:

  • This isn't a proper fix, just a kludge.
  • I could use some kind of find and replace to put this at the top of every file.
  • There are almost certainly other unsanitised SQL input problems.
  • The code should be using prepared statements.

However, it would block 99% of the problems, and gain me some much needed breathing room. If at all possible I would much prefer to do it globally in the PHP config or the like. Many thanks for any suggestions.

EDIT: Below is some code I've knocked together to include via auto prepend, in case it is useful to others. You'll need to replace the database username, password, and possibly the charset too. In addition to $_GET and $_POST, you might also want to sanitise the $_REQUEST, $_SESSION and $_COOKIE superglobals. Debugging code is included for testing purposes. Note that this is intended as a stopgap, not as a substitute for proper input sanitisation! Thanks again to all who offered advice.

<?php
$debug = false;
if( $debug ) ini_set( 'display_errors', 1 );

if( !function_exists( '_sanitise' ) )
{
    function _sanitise( $input )
    {
        return htmlspecialchars( $input, ENT_QUOTES, 'UTF-8' );
    }
}

if( !function_exists( '_sanitise_sql' ) )
{
    function _sanitise_sql( $input )
    {
        $mysqli = new mysqli( 'localhost', 'username', 'password', 'database' );
        /*if( mysqli_connect_error() )
        {
            echo 'Connect Error (' . mysqli_connect_errno() . ') ' . mysqli_connect_error();
        }*/
        $mysqli->set_charset( 'utf8mb4' );
        if( is_array( $input ) )
        {
            $result = array();
            foreach( $input as $key => $value )
            {
                $result[$key] = _sanitise_sql( $value );
            }
            return $result;
        }
        else return $mysqli->real_escape_string( $input );
    }
}

if( !isset( $_GET['_sanitised'] ) )
{
    foreach( $_GET as $key => &$value )
    {
        if( mb_strtolower( $key ) === 'id' ) $value = (int) $value;
        elseif( substr( mb_strtolower( $key ), -3 ) === '_id' ) $value = (int) $value;
        else $value = _sanitise_sql( $value );
    }
    $_GET['_sanitised'] = true;
}

if( !isset( $_POST['_sanitised'] ) )
{
    foreach( $_POST as $key => &$value )
    {
        if( mb_strtolower( $key ) === 'id' ) $value = (int) $value;
        elseif( substr( mb_strtolower( $key ), -3 ) === '_id' ) $value = (int) $value;
        else $value = _sanitise_sql( $value );
    }
    $_POST['_sanitised'] = true;
}

if( $debug )
{
    echo '<pre>$_GET:' . PHP_EOL;
    echo _sanitise( print_r( $_GET, true ) );
    echo PHP_EOL . '$_POST:' . PHP_EOL;
    echo _sanitise( print_r( $_POST, true ) );
    echo '</pre>';
}
Community
  • 1
  • 1
Kitserve
  • 115
  • 9
  • 3
    This is not directly supported by php, but you could the "auto prepend" feature for this. It allows to include a php file at the top of each file requested by a request (so only the "first" php php for each request". In that script you can do such preprocessing. Ugly but working. – arkascha Dec 09 '15 at 13:03
  • It depends on whether you can get away with sanitising all inputs - if you run everything through `mysql_real_escape_string()`, will that cause any issues? For example passing a value from one page to another in the query string will result in backslashed quotes etc. You might be able to get away with escaping based on element names i.e. only apply it if you are sure it is for the database. – halfer Dec 09 '15 at 13:06
  • Possible duplicate of [What's the best method for sanitizing user input with PHP?](http://stackoverflow.com/questions/129677/whats-the-best-method-for-sanitizing-user-input-with-php) – hdost Dec 09 '15 at 13:06
  • @arkascha sorry. I've not read your comment. If you want add your reply I'll delete my answer – Luca Rainone Dec 09 '15 at 13:08
  • Thanks for the comments, folks. Combining auto prepend with `mysql_real_escape_string()` should hold things together temporarily while I'm working on the full review. – Kitserve Dec 09 '15 at 14:01
  • @Kitserve Thanks for the offer, but there is no benefit by that. The issue has been solved, it does not matter by whom the lines were typed that helped. Have fun! – arkascha Dec 09 '15 at 17:38

1 Answers1

2

You can prepend a file for each request with auto-prepend-file in your php.ini

auto_prepend_file = /path/of/your/file.php

and here do every sanitization that you want.

Specifies the name of a file that is automatically parsed before the main file. The file is included as if it was called with the require function, so include_path is used.

Luca Rainone
  • 16,138
  • 2
  • 38
  • 52
  • Many thanks (also to @arkascha), that's pretty much exactly what I was looking for. If this was a new codebase then I would build it with proper user input sanitisation from the beginning, but inheriting it from someone else hasn't left me with that option. Hopefully this answer will be useful as an interim measure to other people who find themselves in a similar situation. – Kitserve Dec 09 '15 at 13:59