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.
Page 1 of 2 12 LastLast
Results 1 to 15 of 17
  1. #1
    New Coder
    Join Date
    Jan 2011
    Posts
    66
    Thanks
    10
    Thanked 5 Times in 5 Posts

    Parametrizing SQL queries

    Ok apologies in advance for my rather vague question but here goes..

    I'm looking to build an essentially very simple website which needn't do much more than log users in, allow them to add records to a very simple database and allow them to view other records too.

    What I have done is modified this login script here: http://www.vclcomponents.com/PHP/Tip...ures-info.html to give me the login system and I have also built on its functions to insert records for viewing.

    Now it is quite old and doesn't seem to do much to prevent SQL injection.

    I have read that the no.1 way of coding against sql injection is to parametize queries. There seems to be very little in the way of information about this and what little information is there seems to vary quite a lot in method/code.

    What I am essentially asking is it A: is it possible to simply convert this code to parametized queries without a complete rewrite. B: How would I do it ? C: is there a more up to date login script (based in php and mysql) floating around which is based around parametrized queries and would be better for me to modify ?

  • #2
    Banned
    Join Date
    Feb 2011
    Posts
    2,699
    Thanks
    13
    Thanked 395 Times in 395 Posts
    Basically all user inputs to a sql query need to first be validated and sanitised before inserting them into an sql query to help prevent sql injection.

    A relatively simple process in preparing user inputs to an sql query is to first validate the input to ensure it contains no invalid characters that could be used to facilitate sql injection and then sanitise the input with mysql_real_escape_string() if using php.

    A better option to the above, although pretty safe if done properly, is to use sql prepared statements

    This link on parameterised queries might also help.
    Last edited by bullant; 05-02-2011 at 02:02 PM.

  • #3
    New Coder
    Join Date
    Jan 2011
    Posts
    66
    Thanks
    10
    Thanked 5 Times in 5 Posts
    Quote Originally Posted by bullant View Post
    Basically all user inputs to a sql query need to first be validated and sanitised before inserting them into an sql query to help prevent sql injection.

    A relatively simple process in preparing user inputs to an sql query is to first validate the input to ensure it contains no invalid characters that could be used to facilitate sql injection and then sanitise the input with mysql_real_escape_string() if using php.

    A better option to the above, although pretty safe if done properly, is to use sql prepared statements
    Thanks for the reply but I'm looking for advice specific to this code.

    I know the basic principles of preventing sql injection and what prepared statements are, I'm just having difficulty with fully understanding if and how it is possible to take the login script I have purloined and work them into it without a complete rewrite.

    by the way, I know I'm kinda asking a lot with this question and I fully expect it to go unasnwered but hey I thought I'd ask!

  • #4
    Banned
    Join Date
    Feb 2011
    Posts
    2,699
    Thanks
    13
    Thanked 395 Times in 395 Posts
    ok no problem

    The link you posted requires you to download the php login code to view it which I haven't done.

    But assuming it accepts a user entered username and password then it should be relatively minimal additional code to first validate the entered username and password and then sanitising them with mysql_real_escape_string, if the code doesn't do that already, before passing the values to either a parameterised or non-parameterised query.

    The last link I posted earlier provides some info on how to use parameterised queries which might help you tweak your code.

  • #5
    New Coder
    Join Date
    Jan 2011
    Posts
    66
    Thanks
    10
    Thanked 5 Times in 5 Posts
    I posted a download link because it's actually quite a large script spread across several files, trying to explain exactly how it's setup would take me so long a person might as well just read the code.

    Its built using classes which is why I think I'm having trouble trying to turn the queries into parameterized queries.

    query's are built using information (connection info etc) setup in other classes and functions and it doesn't seem to be quite so simple to rewrite the code in a DRY manner so that it doesn't break everything else.


    For example this is the function that inserts a new record (I have added this)



    PHP Code:
       function addNewPrediction($userid$prediction$predictiondate){
       
        
    $time time();        
         
          
    $q "INSERT INTO ".TBL_PREDICTIONS." (userid,prediction,predictiondate,predictionmade,flag) VALUES ('$userid', '$prediction', '$predictiondate', '$time', '0')";
          return 
    mysql_query($q$this->connection);
       } 
    Which works but is obviously unsecure.






    Now in an attempt to just get it para entries working in some manner I have added all the connection info into the function when really I want to have that in one place for everything else to access:



    PHP Code:
       function addNewPrediction($userid$prediction$predictiondate){
       
        
    $time time();    
        
        
        
    $db = new mysqli('localhost''root''password''newdb');
        
        
    $stmt $db->stmt_init();
        
        if(
    $stmt->prepare("INSERT INTO ".TBL_PREDICTIONS." (userid,prediction,predictiondate,predictionmade,flag) VALUES (?,?,?,?,?)")) {
        
        
    $stmt->bind_param($userid$prediction$predictiondate$time);
        
        
    $stmt->execute();

        
    $stmt->close();    
        
        };    
          
        
       } 
    That does not work. If I could understand why that doesn't work I think I would be halfway there to understanding even roughly how I need to alter this script.

  • #6
    Regular Coder
    Join Date
    Aug 2006
    Location
    Richmond, CA
    Posts
    221
    Thanks
    3
    Thanked 11 Times in 10 Posts
    Couple things I notice at first glance (and I don't use this stuff much), is that:

    1. First parameter should denote the types you are passing
    2. You are declaring 5 parameters in your statement, but only binding 4.

  • #7
    God Emperor Fou-Lu's Avatar
    Join Date
    Sep 2002
    Location
    Saskatoon, Saskatchewan
    Posts
    16,994
    Thanks
    4
    Thanked 2,662 Times in 2,631 Posts
    The datatypes are unknown. Great work converting to a prepared statement though, this is much better (and implicitly escapes as well) than the old school MySQL. I'll treat it in an OO fashion as you have.
    I'd also recommend re-using a current MySQLi connection (passed via the method parameter) instead of opening it within the function (which if you do, don't open or close the $db).
    PHP Code:
    function addNewPrediction($userid$prediction$predictiondate)
    {    
        
    $time time();    
        
    $db = new mysqli('localhost''root''password''newdb');
        if (!
    mysql_connect_errno()) // Unfortunately, before 5.2.9/5.3.0 the connect_errno member is incorrect
        
    {
        
    // stmt is chained to the prepare, so you can avoid using it if you like:
            
    if ($stmt $db->prepare("INSERT INTO " TBL_PREDICTIONS " (userid, prediction, predictiondate, predictionmade, flag) VALUES (?, ?, ?, ?, ?)"))
            {
                
    $stmt->bind_param("sssss"$userid$prediction$predictiondate$time"0");
                
    $stmt->execute();
                
    $stmt->close();
            }
            
    $db->close();
        }
        else
        {
            throw new 
    Exception(mysqli_connect_error());
        }

    I don't know what datatypes you have used I'm afraid. The earlier example you have indicates that they are all strings, but I doubt that they are. You should use explicit datatyping, since MySQL's implicit conversions is an option of the database itself (I myself configure MySQL to use strict datatyping all the time).
    Last edited by Fou-Lu; 05-03-2011 at 04:21 PM.
    PHP Code:
    header('HTTP/1.1 420 Enhance Your Calm'); 
    Been gone for a few months, and haven't programmed in that long of a time. Meh, I'll wing it ;)

  • Users who have thanked Fou-Lu for this post:

    Jimather (05-03-2011)

  • #8
    New Coder
    Join Date
    Jan 2011
    Posts
    66
    Thanks
    10
    Thanked 5 Times in 5 Posts
    Thanks! will take a closer look at that soon as I can and let you know how I get on. Real life will be disturbing me for a while

  • #9
    New Coder
    Join Date
    Jan 2011
    Posts
    66
    Thanks
    10
    Thanked 5 Times in 5 Posts
    Still can't get that code to work.

    So I'm guessing that the "sssss" is defining the datatypes when binding the parameters there, do you have a reference for exactly which datatypes are which letter ? so I can adjust that and see if it works then.

  • #10
    New Coder
    Join Date
    Jan 2011
    Posts
    66
    Thanks
    10
    Thanked 5 Times in 5 Posts
    PHP Code:
    function addNewPrediction($userid$prediction$predictiondate)
    {   

    $time time();
     
    $mysqli = new mysqli("localhost""root""password""newdb");

    /* check connection */
    if (mysqli_connect_errno()) {
        
    printf("Connect failed: %s\n"mysqli_connect_error());
        exit();
    }

    /* Create the prepared statement */
    if ($stmt $mysqli->prepare("INSERT INTO predictions (userid,prediction,predictiondate,predictionmade) VALUES (?, ?, ?, ?)")) {
        
        
    /* Bind our params */
        
    $stmt->bind_param('isii'$userid$prediction$predictiondate$time);
        

        
    /* Execute the prepared Statement */
        
    $stmt->execute();
        
        
    /* Echo results */
        
    echo "Inserted {$prediction} into database\n"
        
    printf("%d Row inserted.\n"$stmt->affected_rows);
        
        
    /* Close the statement */
        
    $stmt->close();    
        
    }
    else {
        
    /* Error */
        
    printf("Prepared Statement Error: %s\n"$mysqli->error);
    }
        
        
    $mysqli->close();    

    }  

    $userid '57';
    $prediction 'test';
    $predictiondate "9202823233";

    addNewPrediction($userid$prediction$predictiondate); 
    Ok, this is a standalone test script and its close to working but not quite.

    it APPEARS to run successfully and it will dutifully report that it was successful and one row was affected, but.... it hasn't. Nothing new shows up when I look at the table in phpmyadmin.

    However deleting records through the same script works just fine, any ideas anyone ?

  • #11
    Regular Coder
    Join Date
    Oct 2009
    Location
    United States
    Posts
    158
    Thanks
    8
    Thanked 4 Times in 4 Posts
    This can be deleted
    Adobe Dreamweaver shall be destroyed!

  • #12
    New Coder
    Join Date
    Jan 2011
    Posts
    66
    Thanks
    10
    Thanked 5 Times in 5 Posts
    Quote Originally Posted by votter View Post
    This can be deleted
    I don't quite understand


    Oh and if anybody is interested, this reports '-1 row inserted' ?? minus 1 row ???

    PHP Code:
    function addNewPrediction()
    {   

     
    $link mysqli_connect('localhost''root''password''newdb');

    /* check connection */
    if (!$link) {
        
    printf("Connect failed: %s\n"mysqli_connect_error());
        exit();
    }

    $stmt mysqli_prepare($link"INSERT INTO predictions VALUES (?, ?, ?, ?, ?, ?)");
    mysqli_stmt_bind_param($stmt'isiiii'$userid$prediction$predictiondate$time$flag$id);

    $userid '57';
    $prediction 'test';
    $predictiondate "9202823233";
    $time time();
    $flag '0';
    $id NULL;

    /* execute prepared statement */
    mysqli_stmt_execute($stmt);

    printf("%d Row inserted.\n"mysqli_stmt_affected_rows($stmt));

    /* close statement and connection */
    mysqli_stmt_close($stmt);

    /* close connection */
    mysqli_close($link);
    }  



    addNewPrediction(); 

  • #13
    Regular Coder
    Join Date
    Oct 2009
    Location
    United States
    Posts
    158
    Thanks
    8
    Thanked 4 Times in 4 Posts
    My reply, not the topic. lol
    Adobe Dreamweaver shall be destroyed!

  • #14
    New Coder
    Join Date
    Jan 2011
    Posts
    66
    Thanks
    10
    Thanked 5 Times in 5 Posts
    Quote Originally Posted by votter View Post
    My reply, not the topic. lol
    Oh I see lol

  • #15
    New Coder
    Join Date
    Jan 2011
    Posts
    66
    Thanks
    10
    Thanked 5 Times in 5 Posts
    Ah so -1 rows returned means that the query was invalid.

    Now what is invalid about this frickin query , this is driving me insane


  •  
    Page 1 of 2 12 LastLast

    Posting Permissions

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