CODEWITHSUNDEEP
phpget
James
James

Reputation: 43677

PHP safe $_GET or not

We have url:

http://site.com/index.php?action=show

$_GET['action'] is used in templates to check value of ?action=:

switch ($_GET['action']) {
    case = "show" {
        $match_show = true;
    }
}

and in other place:

echo $_GET['action'];

Is it absolutely safe to use this constructions?

How to make them safe?

Thanks.

Upvotes: 13

Views: 23004

Answers (6)

kubik256
kubik256

Reputation: 11

To replace empty (not set) values with default ones try this:

 $variable = isset($_GET['get_variable']) ? $_GET['get_variable'] : 'some-default';

Upvotes: 1

Pekka
Pekka

Reputation: 449783

The switch thing is okay, because you are comparing against a hard-coded value (however, it's case "show": btw).

As @Bruce mentions in the comments, you should add a default: case as well to catch values that are not on the list, or empty values:

switch ($_GET['action']) {

    case "show":
        $match_show = true;
        break;

    default: 
        // value is not on the list. React accordingly.
        echo "Unknown value for 'action'". 

}

The second thing is potentially dangerous, as it would be possible to inject HTML and more importantly, JavaScript into the document body. You should apply a htmlspecialchars() on the variable before echoing it.

Upvotes: 12

Bruce Alderson
Bruce Alderson

Reputation: 1517

The $_GET superglobal is not safe by default as it may contain special or encoded characters and other undesirable text sequences.

You can use the built in PHP function filter_input to sanitize the string according to several standard filters (see the list of filters for an idea of what is possible).

Example:

if (!($action = filter_input(INPUT_GET, 'action', FILTER_SANITIZE_STRING))) {
    $action = 'some-default'; 
}

Advantages:

Relies on built-in sanitize filtering, which ensures:

  • security fixes are available as PHP is patched
  • consistent/standard filtering
  • solid filter implementation (in terms of performance and security)
  • also provides a clear default case

Disadvantages:

  • syntax is a bit of a bear to remember
  • cannot use filter_input if you modify the superglobal $_GET (which you probably shouldn't do anyway).

Side note

You could also check that the field was one of a set using in_array, which is a more dynamic method of checking if you have one of a set.

$search = in_array($search, array('show', 'hide')) ? $search : 'some-default';

The dynamic approach allows you to either execute or look up the target action safely, while storing the set of potential choices in a data structure versus static code.

Upvotes: 9

Flakron Bytyqi
Flakron Bytyqi

Reputation: 3254

PHP $_GET by itself is insecure, it can be exploited in several areas, for a good reading and examples I recommend you to read this article

http://articles.sitepoint.com/article/php-security-blunders

Upvotes: 2

vlad b.
vlad b.

Reputation: 695

Sometimes when i have alot of begginers create plugins or moduldes for a site i use something like...

foreach($_GET as $key=>$value) {
  if(functions_exists('clean_get_'.$key)) {
    $_GET[$key]=call_user_func('clean_get_'.$key,$value);
  } else {
    unset($_GET[$key];
  }
}

... and all the get and post values are 'magically' cleaned or removed so i don't need to worry about someone elses sql-injectable plugin.

Or, if you are a fan of lazy-loading ...

   foreach($_GET as $key=>$value) {
      if(is_file('clean_get_'.$key.'.php')) {
        include_once('clean_get_'.$key.'.php');
        if(functions_exists('clean_get_'.$key)) {
          $_GET[$key]=call_user_func('clean_get_'.$key,$value); 
        } else {
          unset($_GET[$key]);
        }
      } else {
        unset($_GET[$key];
      }
    }

ps. code was written here directly, mistakes are probable!

Upvotes: 2

MrWhite
MrWhite

Reputation: 45968

Yes, as mentioned, you must validate the value of any $_GET variable before using it blindly. But...

You should also be checking that it even exists before using it. Depending on how you have error_reporting() set on your server, if you try to use $_GET['action'] and ?action=something has not been specified in the URL then you'll get an E_NOTICE - Undefined index : action, which will either pollute your error logs or worse, appear in the browser.

$urlAction = isset($_GET['action']) ? $_GET['action'] : null;

if (isset($urlAction)) {
  // Rest of validation...
}

Upvotes: 3

Related Questions