PHP - Smart way to protect $_GET vars from malicio

Question

I found this piece of code here: http://php.net/manual/de/reserved.variables.get.php

Want to use it to make my code safer. I use quite a few $_GET var in my project.

Please, if possible I would like you professionals to have a look and see if this piece of code could be enhanced or has any problems.

There is a smart way to protect the $ _GET input from malicious injection and options for inserting default values:

<?php 
// Smart GET function
public function GET($name=NULL, $value=false, $option="default")
{
    $option=false; // Old version depricated part
    $content=(!empty($_GET[$name]) ? trim($_GET[$name]) (!empty($value) && !is_array($value) ? trim($value) : false));
    if(is_numeric($content))
        return preg_replace("@([^0-9])@Ui", "", $content);
    else if(is_bool($content))
        return ($content?true:false);
    else if(is_float($content))
        return preg_replace("@([^0-9\,\.\+\-])@Ui", "", $content);
    else if(is_string($content))
    {
        if(filter_var ($content, FILTER_VALIDATE_URL))
            return $content;
        else if(filter_var ($content, FILTER_VALIDATE_EMAIL))
            return $content;
        else if(filter_var ($content, FILTER_VALIDATE_IP))
            return $content;
        else if(filter_var ($content, FILTER_VALIDATE_FLOAT))
            return $content;
        else
            return preg_replace("@([^a-zA-Z0-9\+\-\_\*\@\$\!\;\.\?\#\:\=\%\/\ ]+)@Ui", "", $content);
    }
    else false;
}

/*
DEFAULT: $_GET['page'];
SMART: GET('page'); // return value or false if is null or bad input
*/
?>

Source : http://php.net/manual/de/reserved.variables.get.php

Answer 1

The idea is old and often implemented. I personally prefer a variant where you also specify the expected type of argument. Without that I see no point in the approach, since the function can only react to what it gets.

Take the "is_bool" case for example. That does not make any sense at all the way it is implemented now. php will never provide a boolean here, it will always interpret get argument as strings with the content "false" or "true" if specified or 0 and 1 or even an empty string. So this condition will never met.

Take the "is_numeric" case as another example. Certainly it is possible to convert a string variable to a clean numeric type. But what is the idea behind that regex based replacement? If the content results in true as return value of the is_numeric() call, then there certainly are only numbers in there. Per definition. This assumes that what was actually meant here was is_int(), since otherwise that condition branch would block out the two following ones which does not make any sense at all.

What I miss here is the explicit conversion too. Why all the effort if you then do not convert to a clear variable type?

---

This would be different if you specify the expected type in the function:

// returns a boolean
$isSomeFlagSet = GET('bool_flag', 'bool'); 
// returns an integer or throws an exception
$numberOfHits = GET('number_of_hits', 'int'); 
// returns a string, maybe trimmed for convenience
$userName = GET('user_name', 'string'); 

You even an extend that to non builtin types which adds to its value:

// returns a string which is guaranteed to be a valid url
$baseUrl = GET('base_url', 'url'); 
// returns maybe a two char language code (e.g.'en') or throws an exception
$userLang = GET('lang', 'langcode');
// you could add a type 'json' which returns an array from a json get parameter 
$priceMatrix = GET('prices', 'json'); 

In that case a value validation actually does make sense, since it can clearly decide if a conversion of the provided string does make sense. And it can do an explicit and expected conversion so that you have a guaranteed variable type after calling that function.