PHP safe $_GET or not

Question

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.

Answer 1

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

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.

Answer 2

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

Answer 3

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...
}

Answer 4

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!