mahenda Posted September 6, 2019 Share Posted September 6, 2019 //link to the product <a href="<?php echo 'product.php?product_id='. $row['product_id'];?>"style="text-decortion:none;"> //on the product page, the url look like this localhost/maembe/product.php?product_id=2 what will happen when attacker see this id and how to change it Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/ Share on other sites More sharing options...
gw1500se Posted September 6, 2019 Share Posted September 6, 2019 Exactly what are you trying to protect? The answer depends on what 'product.php' does with 'product_id'. Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569428 Share on other sites More sharing options...
chhorn Posted September 6, 2019 Share Posted September 6, 2019 (edited) Nothing will happen as you do not use any variable - except for $row what will raise an undefined variable/undefined index error. didn't you even try this yourself? Edited September 6, 2019 by chhorn Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569429 Share on other sites More sharing options...
mahenda Posted September 6, 2019 Author Share Posted September 6, 2019 13 minutes ago, gw1500se said: Exactly what are you trying to protect? The answer depends on what 'product.php' does with 'product_id'. when user click the link with product picture, the link will open new page called product.php with product full detail from database in the product page the query accepted with get method $product_details = "SELECT * FROM product WHERE product_id=".$_GET['product_id']; Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569430 Share on other sites More sharing options...
chhorn Posted September 6, 2019 Share Posted September 6, 2019 Oh yeah, your database will be deleted then. Hint: Use Prepared Statements. Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569431 Share on other sites More sharing options...
mahenda Posted September 6, 2019 Author Share Posted September 6, 2019 1 minute ago, chhorn said: Nothing will happen as you do not use any variable - except for $row what will raise an undefined variable/undefined index error. i shortened the code assume all variable are available Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569432 Share on other sites More sharing options...
gw1500se Posted September 6, 2019 Share Posted September 6, 2019 (edited) Are you using prepared statements? If so it is not a problem unless you don't want unauthorized users to see product details. In that case you would need to authenticate each user. Edited September 6, 2019 by gw1500se Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569433 Share on other sites More sharing options...
mahenda Posted September 6, 2019 Author Share Posted September 6, 2019 1 minute ago, chhorn said: Oh yeah, your database will be deleted then. Hint: Use Prepared Statements. $prepare = $connect->prepare($product_details); $prepare->execute(); $row = $prepare->fetch(); Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569434 Share on other sites More sharing options...
mahenda Posted September 6, 2019 Author Share Posted September 6, 2019 4 minutes ago, gw1500se said: Are you using prepared statements? If so it is not a problem unless you don't want unauthorized users to see product details. In that case you would need to authenticate each user. every user can see, even if he/she did not logged in Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569435 Share on other sites More sharing options...
mahenda Posted September 6, 2019 Author Share Posted September 6, 2019 3 minutes ago, mahenda said: $prepare = $connect->prepare($product_details); $prepare->execute(); $row = $prepare->fetch(); is this correct Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569436 Share on other sites More sharing options...
gw1500se Posted September 6, 2019 Share Posted September 6, 2019 Yep. Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569437 Share on other sites More sharing options...
ginerjm Posted September 6, 2019 Share Posted September 6, 2019 Spelling error here.... style="text-decortion:none;" Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569438 Share on other sites More sharing options...
Barand Posted September 6, 2019 Share Posted September 6, 2019 56 minutes ago, gw1500se said: Yep Not if $product_details is still as posted earlier IE 1 hour ago, mahenda said: $product_details = "SELECT * FROM product WHERE product_id=".$_GET['product_id']; You need $product_details = "SELECT * FROM product WHERE product_id = ?"; $prepare = $connect->prepare($product_details); $prepare->execute( [ $_GET['product_id'] ] ); Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569439 Share on other sites More sharing options...
gw1500se Posted September 6, 2019 Share Posted September 6, 2019 I didn't think there was a difference between the 2. In any case one should always validate the data before using it. Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569441 Share on other sites More sharing options...
Barand Posted September 6, 2019 Share Posted September 6, 2019 5 minutes ago, gw1500se said: I didn't think there was a difference between the 2 The principle behind prepared statements is the separation of user-provided data from the query SQL code. This is accomplished by putting placeholders in the query and then binding parameters to those placeholders when executing Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569442 Share on other sites More sharing options...
gw1500se Posted September 6, 2019 Share Posted September 6, 2019 Not to hijack this thread, but isn't the preparation process the same in either case? Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569443 Share on other sites More sharing options...
Barand Posted September 6, 2019 Share Posted September 6, 2019 Not even close. This code... $product_details = "SELECT * FROM product WHERE product_id=".$_GET['product_id']; $prepare = $connect->prepare($product_details); $prepare->execute(); ...would embed any SQL injection code contained in the GET into the query which would then be executed. (Just as an unprepared query would) In the correct version the injection code would only be treated as data and not part of the SQL code. 1 1 Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569444 Share on other sites More sharing options...
gw1500se Posted September 6, 2019 Share Posted September 6, 2019 Thanks. Learned something new. So in my case the prepare is really useless. Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1569445 Share on other sites More sharing options...
mahenda Posted September 27, 2019 Author Share Posted September 27, 2019 On 9/6/2019 at 5:15 PM, ginerjm said: Spelling error here.... style="text-decortion:none;" style="text-decoration:none;" Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1570008 Share on other sites More sharing options...
mahenda Posted September 27, 2019 Author Share Posted September 27, 2019 On 9/6/2019 at 5:44 PM, Barand said: Not if $product_details is still as posted earlier IE You need $product_details = "SELECT * FROM product WHERE product_id = ?"; $prepare = $connect->prepare($product_details); $prepare->execute( [ $_GET['product_id'] ] ); thank you so much but why no bindParam() Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1570009 Share on other sites More sharing options...
Barand Posted September 27, 2019 Share Posted September 27, 2019 1 minute ago, mahenda said: but why no bindParam() Because I preferred to pass the parameters in an array when executing instead. Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1570010 Share on other sites More sharing options...
mahenda Posted September 27, 2019 Author Share Posted September 27, 2019 8 minutes ago, Barand said: Because I preferred to pass the parameters in an array when executing instead. okey thanks Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1570014 Share on other sites More sharing options...
mahenda Posted September 27, 2019 Author Share Posted September 27, 2019 11 minutes ago, Barand said: Because I preferred to pass the parameters in an array when executing instead. let me return back again is saw something like localhost/maembe/product.php?product_id/2 or this localhost/maembe/product.php?product/hot-coffee-found-here when i click on the home link but i have no idea on how to do that my own is localhost/maembe/product.php?product_title = hot-coffee-found-here i dont want that '=' sign if i'll use preg_replace() will be collect isn't it or anyway ? Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1570015 Share on other sites More sharing options...
Barand Posted September 27, 2019 Share Posted September 27, 2019 (edited) 55 minutes ago, Barand said: Because I preferred to pass the parameters in an array when executing instead. Binding is useful when you want to process records in a loop. Bind the variables first then, in the loop, update the values and execute. EG $data = [ [ 1, 'Curly'], [ 2, 'Larry'], [ 3, 'Mo'] ]; $stmt = $db->prepare("INSERT INTO testuser (id, username) VALUES (:id, :user)"); $stmt->bindParam(':id', $id, PDO::PARAM_INT); $stmt->bindParam(':user', $username, PDO::PARAM_STR); foreach ($data as $user) { list($id, $username) = $user; $stmt->execute(); } EDIT: But, with PDO, there is the alternative that I used before EG $data = [ [ 1, 'Curly'], [ 2, 'Larry'], [ 3, 'Mo'] ]; $stmt = $db->prepare("INSERT INTO testuser (id, username) VALUES (?, ?)"); foreach ($data as $user) { $stmt->execute($user); } where the values are passed as an array when executing. Edited September 27, 2019 by Barand 1 Quote Link to comment https://forums.phpfreaks.com/topic/309191-is-this-one-vulnerable/#findComment-1570016 Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.