SitePoint Sponsor

User Tag List

Results 1 to 7 of 7
  1. #1
    SitePoint Evangelist stef25's Avatar
    Join Date
    Nov 2004
    Location
    belgium
    Posts
    465
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    conditional statements and $_GET

    the if statement below builds an sql query based on which parameter was passed through the query string. this runs on my shop.php page and the first part works fine: if some one requests shop.php?cat=games, it pulls all games from the db.

    if people simply request shop.php i want all products to show. i could do this using shop.php?cat=all but would rather just have it work with shop.php cause people could (and will) type this in themselves

    why does the else statement not work? i thought it should trigger that query when the conditions in the if statement are false?

    Code:
    $queryVar = $_GET['cat'];
    
    if ($queryVar == 'games' || 'accessories' || 'clothing') {
    	$prodQuery = "SELECT * FROM products WHERE category = '$queryVar'";
    	$prodResult = mysql_query($prodQuery) or die ("couldnt grab the product data");	
    }
    
    else {
    	$prodQuery = "SELECT * FROM products";
    	$prodResult = mysql_query($prodQuery) or die ("couldnt grab the product data");	
    	
    }
    the second part of my question is - will this "catch all" else statement prevent injection attacks by just showing all products even if ppl put funny code in to my url?
    I need someone to protect me from
    all the measures they take in order to protect me

  2. #2
    I meant that to happen silver trophybronze trophy Raffles's Avatar
    Join Date
    Sep 2005
    Location
    Tanzania
    Posts
    4,662
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    It's because you're constructing the if statement wrongly. The way you have it, the second and third bits evaluate to true, so the if statement always will evaluate to true, so the else part will never be reached. It reads like this: "if $queryVar is 'games' OR if 'accessories' is something OR if 'clothing' is something. Clearly 'accessories' and 'clothing' are something - they're strings. So you have to do this:
    PHP Code:
    if ($queryVar == 'games' || $queryVar == 'accessories' || $queryVar == 'clothing') { 
    From what I can see, there is no way a malicious person could do any nasty injecting because you've limited it to three distinct strings and you're not using raw user input (which should never be done).

  3. #3
    SitePoint Member
    Join Date
    Nov 2006
    Posts
    1
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    In that way it will work:

    Code:
    if ($queryVar == 'games' || $queryVar == 'accessories' || $queryVar == 'clothing') {...}
    IMHO this will be save from sql-injections ...

  4. #4
    SitePoint Evangelist stef25's Avatar
    Join Date
    Nov 2004
    Location
    belgium
    Posts
    465
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    thanks, works perfectly
    I need someone to protect me from
    all the measures they take in order to protect me

  5. #5
    SitePoint Wizard silver trophybronze trophy Cups's Avatar
    Join Date
    Oct 2006
    Location
    France, deep rural.
    Posts
    6,869
    Mentioned
    17 Post(s)
    Tagged
    1 Thread(s)
    Heres a couple of ideas

    PHP Code:
    $queryVar $_GET['cat'];

    // easier to manage a big list like this for now

    $allowed=array(
    'games',
    'accessories',
    'clothing',
    )

    // if the var is not even set, do the default action

    if ( ( isset($queryVAr ) ) && ( in_array($queryVar $allowed) )  {
        
    $prodQuery "SELECT * FROM products WHERE category = '$queryVar'";
    }

    else {
        
    $prodQuery "SELECT * FROM products";
        
    }

    // you do this either way
        
    $prodResult mysql_query($prodQuery) or die ("couldnt grab the product data"); 


    the second part of my question is - will this "catch all" else statement prevent injection attacks by just showing all products even if ppl put funny code in to my url?
    Using above you are making a White List of allowed parameters, so the only sql is that which you allow in - so sql injection should not come into play.


    Thats all untested by the way

  6. #6
    I meant that to happen silver trophybronze trophy Raffles's Avatar
    Join Date
    Sep 2005
    Location
    Tanzania
    Posts
    4,662
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    If you end up copy-pasting Cups' suggestion, watch out because there is a small typo in ( isset($queryVAr ).

  7. #7
    SitePoint Wizard silver trophybronze trophy Cups's Avatar
    Join Date
    Oct 2006
    Location
    France, deep rural.
    Posts
    6,869
    Mentioned
    17 Post(s)
    Tagged
    1 Thread(s)
    Thanks for that Raffles, had to just post that off hurriedly ...


Bookmarks

Posting Permissions

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