1. Advertising
    y u no do it?

    Advertising (learn more)

    Advertise virtually anything here, with CPM banner ads, CPM email ads and CPC contextual links. You can target relevant areas of the site and show ads based on geographical location of the user if you wish.

    Starts at just $1 per CPM or $0.10 per CPC.

Buying $5USD PHP programming question

Discussion in 'Programming' started by Manald, Oct 8, 2014.

  1. #1
    Can anyone answer this PHP programming question?

    What is the security flaw in the following line of code and how would you fix it?
    $sql = "UPDATE products SET `name` = '" . $_GET["name"] . "', `price` = '" . $_GET["price"] . "' WHERE id = " . $_GET["id"] . ";";

    The first person that gets the right answer will get $5usd via paypal!

    Best,
    Ormus
     
    Manald, Oct 8, 2014 IP
  2. strike2867

    strike2867 Member

    Messages:
    22
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    36
    As Seller:
    100% - 0
    As Buyer:
    100% - 0
    #2
    http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php

    You can keep the $5.
     
    strike2867, Oct 8, 2014 IP
  3. Fracisc

    Fracisc Well-Known Member

    Messages:
    3,670
    Likes Received:
    10
    Best Answers:
    1
    Trophy Points:
    195
    As Seller:
    100% - 1
    As Buyer:
    50.0% - 1
    #3
    Well, first of all that script should be run behind a protected are. Otherwise everyone could run it. To answer your question, I would use escape string.
     
    Fracisc, Oct 8, 2014 IP
  4. Manald

    Manald Well-Known Member

    Messages:
    809
    Likes Received:
    8
    Best Answers:
    0
    Trophy Points:
    140
    As Seller:
    100% - 0
    As Buyer:
    100% - 0
    #4
    Ok, post the answer here. The first one that post the correct answer will get the money via paypal + job offer for 100x more pay.
     
    Manald, Oct 8, 2014 IP
  5. Fracisc

    Fracisc Well-Known Member

    Messages:
    3,670
    Likes Received:
    10
    Best Answers:
    1
    Trophy Points:
    195
    As Seller:
    100% - 1
    As Buyer:
    50.0% - 1
    #5
    $sql = "UPDATE products SET `name` = '" . mysql_real_escape_string($_GET["name"]) . "', `price` = '" . mysql_real_escape_string($_GET["price"]) . "' WHERE id = " . $_GET["id"] . ";";
     
    Fracisc, Oct 8, 2014 IP
  6. strike2867

    strike2867 Member

    Messages:
    22
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    36
    As Seller:
    100% - 0
    As Buyer:
    100% - 0
    #6
    You should use parameters instead of escaping the string. It will have better performance in most databases and is a better practice.
     
    strike2867, Oct 8, 2014 IP
  7. 14web_net

    14web_net Greenhorn

    Messages:
    7
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    21
    As Seller:
    100% - 0
    As Buyer:
    100% - 0
    #7
    you need the parameter inside the query to make it more secure. here is to fix it :
    <?php
    $host = "localhost";
    $db_name = "yourdb";
    $username = "root";
    $password = "";

    try {
    $con = new PDO("mysql:host={$host};dbname={$db_name}", $username, $password);
    }catch(PDOException $exception){ //to handle connection error
    echo "Connection error: " . $exception->getMessage();
    }

    $query = "UPDATE products SET name = :name , price = :sprice
    WHERE id = :id";

    $stmt = $con->prepare( $query );
    //bind the parameters
    $stmt->bindParam(':name', $_GET["name"]);
    $stmt->bindParam(':sprice', $_GET["price"]);
    $stmt->bindParam(':id', $_GET["id"]);


    $stmt->execute();
    ?>

    I have tested this and work :)
     
    Last edited: Oct 8, 2014
    14web_net, Oct 8, 2014 IP
  8. Feral

    Feral Active Member

    Messages:
    70
    Likes Received:
    10
    Best Answers:
    0
    Trophy Points:
    68
    As Seller:
    100% - 0
    As Buyer:
    100% - 0
    #8
    The question is a little convoluted without enough actual information to answer it properly.
    First off what type of database are you using? MySQL, MSSQL, PostgreSQL, SQLight..
    Secondly what type of connection are you using? Legacy mysql, mysqli, pdo...

    Regardless of the above questions the correct answer would be to use prepared statements of some sort, the function calls used would be determined by the answer to the questions above.
     
    Feral, Oct 8, 2014 IP
  9. creativeGenius

    creativeGenius Well-Known Member

    Messages:
    273
    Likes Received:
    5
    Best Answers:
    1
    Trophy Points:
    120
    As Seller:
    100% - 0
    As Buyer:
    100% - 0
    #9
    the fact that he's getting parameters directly from $_GET is the most obvious security flaw, anyone can type the parameters in the address bar and the records will be vulnerable, regardless of escaping and prepared sql statements.
     
    creativeGenius, Oct 10, 2014 IP
  10. Fracisc

    Fracisc Well-Known Member

    Messages:
    3,670
    Likes Received:
    10
    Best Answers:
    1
    Trophy Points:
    195
    As Seller:
    100% - 1
    As Buyer:
    50.0% - 1
    #10
    Yes, but we have this: SET `name` = '" . $_GET["name"] . "', `price` = '" . $_GET["price"] . "'

    We set an existing value, so the $_GET["name"] should be safe and then, considering that we are talking about a price, we can accept only number as a value for the "price" variable. It should be safe enough.
     
    Fracisc, Oct 10, 2014 IP
  11. creativeGenius

    creativeGenius Well-Known Member

    Messages:
    273
    Likes Received:
    5
    Best Answers:
    1
    Trophy Points:
    120
    As Seller:
    100% - 0
    As Buyer:
    100% - 0
    #11
    i dont know what you mean by setting an existing value, but $_GET always gets the value from a querystring, im not even worried about the correct values for your fields right now, its this line that is most dangerous

    WHERE id = $_GET["id"]

    any user, can manipulate the id variable using just the address bar and update/mess any of your products, and that is the tip of the iceberg, if all your queries are constructed like that, i cant even begin to explain, how easy for it for anyone to completely mess up your website
     
    creativeGenius, Oct 10, 2014 IP
  12. Feral

    Feral Active Member

    Messages:
    70
    Likes Received:
    10
    Best Answers:
    0
    Trophy Points:
    68
    As Seller:
    100% - 0
    As Buyer:
    100% - 0
    #12
    creativeGenius,

    I agree that these variables should not be being passed using query strings. An insert or update query should never use them, but given that I have seen it in production code many times I can't say that this would come as a surprise to me.

    In a case like this you would have to assume that before this code there are at lease security checks to make sure the user has the correct privileges to update the product in this way to begin with. Other than that you still need to secure the variables to prevent injection attacks, that they are even of the correct type for those database fields or that the variables are simply not empty.

    Sometimes in real world situations you just have to run with what your given and make the best of it. So even if they are passing the variables through $_GET there are still many security precautions that can be taken to prevent this from being a real security issue.
     
    Feral, Oct 10, 2014 IP
  13. OaldDesign

    OaldDesign Active Member

    Messages:
    448
    Likes Received:
    11
    Best Answers:
    0
    Trophy Points:
    60
    As Seller:
    100% - 0
    As Buyer:
    100% - 1
    #13
    I truly don't understand why you open this stupid trades and waving $100 job you have.
    Answering a simple question surly don't represent the quality of a coder and no good programmers will waste his time on this questions game so do yourself a favor and if you looking for good coder post the info for the $100 work and test the programmers right now you are wasting your time and the other members time (that is if you really have $100 job).

    anyway i wish you good luck with your projects!
     
    OaldDesign, Oct 16, 2014 IP