1. This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn More.

Development Protecting Against SQL Injection

Discussion in 'Software' started by GuardianStorm, 20 Jul 2007.

  1. GuardianStorm

    GuardianStorm Active Member

    Joined:
    26 Apr 2005
    Posts:
    1,475
    Likes Received:
    1
    Hey guys,

    I have a web page that has a list of options that the user can click on, and it will then query the db based on the click. the links point back to the same page, with each link like this:

    img.php?gal=Name

    the page will then check if its set, then query the db:

    PHP:
    $SQL " SELECT Name, Description FROM Images";                //default value
                    
    if ( isset($_REQUEST['gal']) )
    {
        if ( 
    $_REQUEST['gal'] !== 'All')
        {
            
    $SQL " SELECT i.Name, i.Description FROM Images i, Galleries g WHERE (i.ID = g.ID) AND (g.Name ='" $_REQUEST['gal'] ."')"
        }
    }
    The thing I'm worries about is someone putting in a bad parameter to inject SQL into the db. I had thought of testing to see if the perameter is already in the db, by getting a list of the galleries, and checking if the parameter is in that list, but I'm not too sure how to do this in php.

    PHP:
    $s "SELECT Name FROM Galleries WHERE Name = '" .  $_REQUEST['gal'] . "' "
    $ret mysql_db_query($db$s$dbh);
    (then test if the result set is greater than 0)

    but this wouldn't work as it would still have SQL injection vulnerability.

    any ideas would be great

    thanks
     
  2. DougEdey

    DougEdey I pwn all your storage

    Joined:
    5 Jul 2005
    Posts:
    13,933
    Likes Received:
    33
    Firstly, avoid using $_REQUEST, it's very very unsafe.

    Set the forms to use method="post", then use $_POST array rather then $_REQUEST, that means that it won't be parsed in the url (the URL parsing is $_GET)

    Easiest way to check for injection attacks is to use

    PHP:
    if(stristr(';',$_POST['gal']))
    {
    die(
    'OI! Stop trying to hack me!');
    }
     
  3. RTT

    RTT #parp

    Joined:
    12 Mar 2001
    Posts:
    14,120
    Likes Received:
    74
    Except for it'd only catch just one of a very few ways of attempting SQL injection, so don't do that... :duh:

    GuardianStorm, your code is definitely vulnerable to SQL Injection. If you know all of the names of your galleries you can write some rudimentary protection, but it'd need to be modified every time you added, renamed or removed a gallery in your database.

    The best thing to do is to change the way that you are using PHP to talk to your MySQL server. The MySQLi extension has 'built in' protection against SQL injection via use of prepared statements and isn't all that more difficult to use. See php.net/mysqli and php.net/mysqli_bind_param.
     
  4. DougEdey

    DougEdey I pwn all your storage

    Joined:
    5 Jul 2005
    Posts:
    13,933
    Likes Received:
    33
    The really easiest way, is that if you're expecting a name, which has a maximum length of say 10 characters (or even use a lookup table with converts a number to the gallery) user regular expressions with "[0-9]{*,5}" which checks for just a input of 0-5 numbers, no more nothing else.
     
  5. Ben

    Ben New Member

    Joined:
    11 Aug 2003
    Posts:
    1,000
    Likes Received:
    0
    Would it not be easier and safer to use the gallery id instead of the gallery name, then using intval() to strip out unwanted data? Or limit the gallery names to a-z0-9 and use ctype_alnum() to check the name is valid?
     
    Last edited: 20 Jul 2007
  6. GuardianStorm

    GuardianStorm Active Member

    Joined:
    26 Apr 2005
    Posts:
    1,475
    Likes Received:
    1
    Thanks, but as its not a form (my forms already use $_POST) i will use $_GET.

    Thanks for the link, however as the server is not mine (i rent from Hosting-Unlimited) i dont know if they have mysqli installed or iff they can. is there a script i can run to determine if it is installed/enabled?

    I could use the integer version, as the table is a Name-Value pair, so that would be a very viable option.

    That might be coupled with DougEdey's suggestion, thanks.

    Thanks everyone :)
     
  7. RTT

    RTT #parp

    Joined:
    12 Mar 2001
    Posts:
    14,120
    Likes Received:
    74
    Yep! Although i'd say it'll be easier for you to go down doug's route (integers rather than names) rather than move to mysqli.

    But anyway, here's a script to tell you if they have mysqli or not:

    Code:
    <?php
    echo function_exists('mysqli_connect') ? 'this server has mysqli' : 'this server doesn\'t have mysqli :(';
    ?>
     
  8. GuardianStorm

    GuardianStorm Active Member

    Joined:
    26 Apr 2005
    Posts:
    1,475
    Likes Received:
    1
    thanks for the help again RTT, and unfortunately the server does not have MySQLi installed :(

    I will implement the integer method as suggested by Doug, should be easy enough :)

    thanks again everyone!
     
  9. DougEdey

    DougEdey I pwn all your storage

    Joined:
    5 Jul 2005
    Posts:
    13,933
    Likes Received:
    33
    See, reg ex is brilliant.
     
  10. GuardianStorm

    GuardianStorm Active Member

    Joined:
    26 Apr 2005
    Posts:
    1,475
    Likes Received:
    1
    Thanks for the help everyone, here is the code i am now using:

    PHP:
    if ( isset($_GET['gal']) && (is_numeric($_GET['gal'])) ) 
    {
        
    $SQL "SELECT i.Name, i.Description FROM Images i JOIN ImageGalleries ig ON i.ID = ig.ImageID WHERE ig.GalleryID = '" $_GET['gal'] . "'";
    }
    If anyone can see problems with this, I am all ears :)
     
  11. capnPedro

    capnPedro Hacker. Maker. Engineer.

    Joined:
    11 Apr 2007
    Posts:
    4,381
    Likes Received:
    241
    Can you do parameterized queries? Or is that not doable with PHP or with MySQL or your server? I don't know. I'll find out when I am more sober.
    thanks you.
     
  12. fri2219

    fri2219 New Member

    Joined:
    26 May 2007
    Posts:
    34
    Likes Received:
    0
  13. jezmck

    jezmck Well-Known Member

    Joined:
    25 Sep 2003
    Posts:
    4,456
    Likes Received:
    36
    that's no way to learn!
    do it by hand at first, move to library code later.
     
  14. DougEdey

    DougEdey I pwn all your storage

    Joined:
    5 Jul 2005
    Posts:
    13,933
    Likes Received:
    33
    Guardianstorm: not a major one, but to improve efficiency only use doubles quotes (") to enclose a string if you want PHP to evaluate it (i.e. if you have any escape, \*, chars in there), for non evaluated, static strings, enclose them in single quotes (') not a biggie, but good habits survive a lifetime, when bad habits die slowly
     

Share This Page