1

I'm sending GET strings from another webpage which is mydomain.com/read_data.php?symbol=abc.

Below is my code. Are they secure enough to prevent XSS and other code-related security issues?

if ( $_SERVER['REQUEST_METHOD'] == 'GET' ) {

    if ( isset( $_GET['symbol'] ) && filter_var($_GET['symbol'], FILTER_SANITIZE_STRING) ) {
        $cur_name = filter_var($_GET['symbol'], FILTER_SANITIZE_STRING);
        
    } else {
        
        // redirect them to your homepage
        header('location: /');
        exit;
    }

} else {
    
    // redirect them to your homepage
    header('location: /');
    exit;
}

Or should I be using FILTER_SANITIZE_SPECIAL_CHARS?

ratib90486
  • 89
  • 1
  • 7
  • Are all the symbols that may be passed known to you before-hand? The most secure way is to have a whitelist of accepted symbols and rejecting everything else. – Jacob Mulquin Dec 20 '21 at 03:32
  • `FILTER_SANITIZE_SPECIAL_CHARS` would be better. Note: `FILTER_SANITIZE_STRING` is deprecated in` PHP 8` also you can read [this](https://www.php.net/manual/en/filter.filters.sanitize.php) – Faesal Dec 20 '21 at 03:35
  • 2
    Sanitization is not a blanket for security. Context is very, very important. Looking solely at the code posted, I would say _not_ to do that, because I don’t know how that variable used. – Chris Haas Dec 20 '21 at 03:37
  • If you don't want any HTML, use `strip_tags()` or `htmlspecialchars()`. If you want to use some HTML, use [HTML Purifier](http://htmlpurifier.org/) or other similar class. – vee Dec 20 '21 at 03:45
  • Read more about XSS on [OWASP](https://owasp.org/www-community/attacks/xss/) website, they included [cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html) for you to test your prevention. – vee Dec 20 '21 at 03:48
  • @vee But I thought FILTER_SANITIZE_STRING already strip away any HTML and special chars? – ratib90486 Dec 20 '21 at 05:27
  • @JacobMulquin Yes, all are known. But the list has 570 names and symbols. I don't think it's efficient to list them all. – ratib90486 Dec 20 '21 at 05:28
  • @ratib90486 Please read the [document](https://www.php.net/manual/en/filter.filters.sanitize.php). That filter constant is deprecated since PHP 8.1. If you planned to never go up to PHP 8.1 then you are free to use. If you planned to upgrade to PHP 8.1 now or in the future then please don't use it. – vee Dec 20 '21 at 05:36
  • @vee I read the link you posted. But what are examples of "UNTRUSTED DATA" mentioned in the document? First guess is user input, but is data entered by me from another PHP file or on the same PHP file considered untrusted? – ratib90486 Dec 20 '21 at 05:37
  • 1
    What you type directly inside your script, is not "untrusted." (Although it might still warrant escaping - as already said, _context_ is important.) But for example any GET parameters, like in your question, are of course _not_ trusted. Just because _you_ linked to `yourscript.php?parameter=allowedvalue` somewhere in your code, does not mean I could not easily call `yourscript.php?parameter=somethingelse` in my browser ... – CBroe Dec 20 '21 at 07:50
  • Is this adequate? For the code you've posted it's adequate, and, in fact, unnecessary because you're not doing anything with the strings after you sanitize them. In real code this sort of sanitisation ranges from irrelevant, through adequate to downright harmful depending on what this data is to be used for. There is no single solution to this problem. You must use the solution appropriate for the circumstances. – Tangentially Perpendicular Dec 20 '21 at 08:16
  • 1
    Sanitisation should be done at _output_ time, not input. whether content is a security risk depends on what you're doing with it. E.g. html content can't be an XSS risk when it's sitting in a database, or being saved to a CSV file, or whatever. It's only a risk if you decide to output it into a live HTML document. Assess the risk of the place where you're sending data during output, and sanitise it appropriately at that moment of output, not before. Otherwise, you risk losing information which might have been valid or useful in another context. – ADyson Dec 20 '21 at 08:40
  • @ADyson gotcha! You guys should leave your comments as answers I will duly mark it as answered or upvote. Thanks you guys! – ratib90486 Dec 20 '21 at 11:51
  • Actually I think it's pretty much a duplicate of this: [How can I sanitize user input with PHP?](https://stackoverflow.com/questions/129677/how-can-i-sanitize-user-input-with-php), so no real need to re-post similar answers. Glad the comment was helpful though :-) – ADyson Dec 21 '21 at 22:52

0 Answers0