Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Results 1 to 5 of 5
  1. #1
    Regular Coder
    Join Date
    Feb 2007
    Location
    London
    Posts
    225
    Thanks
    16
    Thanked 2 Times in 2 Posts

    $_GET data type validation

    I want to decide which data to display by checking a URL query string for an id.

    The links are of the form:

    PHP Code:
    echo "<a href='".$_SERVER['SCRIPT_NAME']."?id=".$i."'>BLAH</a>"
    where $i is just an incrementing integer in a for loop.

    I'm wondering, for security reasons, whether it's necessary to validate the data type before doing stuff with it. Is there any potential harm a hacker could do by typing some script into the URL in place of the value of 'id' that I'm expecting to be a number?

    So, esentially, what I'm asking is: is it overkill to validate that the $_GET data is numeric, like so...?

    PHP Code:
      if (isset($_GET['id']) && is_numeric($id)) {
        
    //echo the data in some array where key == $id
      

    I guess that the answer is that if I'm only planning on echoing something in an array, then the worst that could happen is that if the 'id' of the query string isn't numeric, then nothing will be printed, and I'll get an error, but nothing worse could happen.

    Am I underestimating the risks?
    Is such validation good practice regardless?
    What do you guys do?

    Cheers.

  • #2
    Senior Coder
    Join Date
    Jul 2009
    Location
    South Yorkshire, England
    Posts
    2,318
    Thanks
    6
    Thanked 304 Times in 303 Posts
    Quote Originally Posted by cfructose View Post
    Is such validation good practice regardless?
    Always.

  • #3
    Regular Coder
    Join Date
    Feb 2007
    Location
    London
    Posts
    225
    Thanks
    16
    Thanked 2 Times in 2 Posts
    Short but sweet!

    OK, in that case, I suppose I should also check that the number falls within the range I'm expecting...

    Given that the 'id' of the query string will always have a value that's <= the size of my array, I could do:
    PHP Code:
      $validated = (isset($_GET['id']) && is_numeric($_GET['id']) && $_GET['id'] <= count($arr)) ? TRUE FALSE
    ...and then proceed if $validated == TRUE.

    Is this really standard practice? Wow!

    I'm curious though, what would the potential dangers be? I'm not sure I see any. (While I'm more than happy to take your advice and code this way "ALWAYS", I'd like to have a better understanding of what the risks are if I don't).

  • #4
    Senior Coder
    Join Date
    Jul 2009
    Location
    South Yorkshire, England
    Posts
    2,318
    Thanks
    6
    Thanked 304 Times in 303 Posts
    Two simple examples which spring to mind.

    1) If $id was used in a DB query and was unsanitised and unescaped, you have potential for a SQL injection.

    2) If $id was a filepath for an include, an unsanitised/unvalidated var would have the possibility for a directory traversal vulnerability.


    Checking against an array key, as in your case, is fairly harmless. It will either match or it won't, but you should still validate it as being an integer at minimum. For correctness if nothing else. The primary reason for draconian validation and sanitisation at all times is to make sure that you don't forget at some point. If you're always draconian with your input, the chance of introducing a vulnerability/exploit accidentally at some point in the future becomes reduced drastically.

  • #5
    Regular Coder
    Join Date
    Feb 2007
    Location
    London
    Posts
    225
    Thanks
    16
    Thanked 2 Times in 2 Posts
    SQL injection attacks if the var is for a database is a risk I'm well acquainted with. The filepath idea, however, is one that I wouldn't have thought of. I appreciate the examples.

    the primary reason for draconian validation and sanitisation at all times is to make sure that you don't forget at some point.
    Nicely put. Henceforth, my middle name shall be 'Draconian'.


  •  

    Posting Permissions

    • You may not post new threads
    • You may not post replies
    • You may not post attachments
    • You may not edit your posts
    •