Safe $_GET

[Taiphoz]

Hello all.

 

Just wanted to run this past you guys to see if I am missing anything important. I am making a script that I plan to allow a lot of other people around the web to use, so I want to make sure it's as bullet proof as possible.

 

I am passing two values and grabbing them with a _GET, one is a big number, and the other is only letters and 8 characters long.

 

her's my code so far.

 

<?php
    $clan = $_GET['clanid'];
    // make sure its an INT
        //if(isint($clan)){
        if(ereg("[^0-9]", $clan)){
            //im an int.
            echo ("ERROR Invalid CLANID");
            die;          
        }
    
    // make sure its a 8 letter only word.
    $style=$_GET['style'];
        
        // cut style down to 8 characters long.
        $style=substr($style, 0, ;
        
        if(ereg("[^a-zA-Z]+", $style)) {
            // Contains only letters.
            echo("ERROR Invalid STYLE NAME");
            die;          
        }   
    
?>

 

to my noob php eye's it looks pretty solid, I cant think of any way a malicious user could get past it, but like I said, thought I would run it past you guys first , you can never be to careful.

[Zurev]

Well ereg is deprecated as far as my knowledge, are you developing on a platform before PHP5?

 

Google "php type juggling", you'll see you can typecast that $_GET["clanid"]. So this:

$clan = (int)$_GET["clanid"];

 

If someone were to enter a string, it would immediately typecast it to 0. If it's an integer value it would return the integer value, so that's a simple way to get rid of the first regular expression.

 

As far as the $_get[style], you should think if possible, can you whitelist? Meaning if there's only 10 things it can be, create an array of those 10 items and force it to be in that array of allowed items, if you can't whitelist, then what you're doing is fine, except throw in a {8} afterwards to force it to be 8 characters if it's always going to be, and again, use preg_match. You could also look at the sanitizing method and preg_replace anything that isn't a-zA-Z.

[Taiphoz]

preg match stuff looked a bit more complex to wrap my head around which is why I went with ereg, it looked simpler to understand.

[Zurev]

It shouldn't be too far, the expressions should be similar if not the same, I never messed with any POSIX though.

 

So for example, this would check if it's a string 1-8 characters of only a-zA-Z:

$isValid = preg_match("/[a-zA-Z]{1,8}/", $_GET["style"]);

[gizmola]

Zurev has provided some really good advice -- casting to (int) is the best way to handle the integer parameter.  The ereg routines only were deprecated recently, but with that said, the core of the ereg and preg_ routines are both the same -- regex.  The main difference is that in the preg routines you need to add a delimitter around your regex.  In his example he used the backslash, which is a common approach but you can use a different character if it suits you. Otherwise, it's not a big deal to take the regex you developed and tested and stick it inside: '/  /'

 

Since ereg has just been deprecated, you might as well bite the bullet and familiarize yourself with the preg_ routines going forward.

[Taiphoz]

yup gona use those..

 

but not just now, i have not actually been to sleep yet lol, and its now time to get the kids ready for school, so im gona go do that then get some sleep, and come back later and read all of this again.

 

thanks guys you really helped me a lot tonight.