Preventing users manipulating URL strings
If a user clicks on a particular record, it shows them their voucher number. Before I added the check to validate the user's ID, I could manipulate the oid value in the URL to any number and if lucky, I could see a voucher from another order. This is obviously not good. Anyway, the code I put together seems to prevent this but I just wanted to check if this is an acceptable method or if there is something better.
function display_vc($link) {
if(isset($_GET['pid']) && isset($_GET['oid'])) {
$pid = filter_var($_GET['pid'], FILTER_SANITIZE_URL);
$oid = filter_var($_GET['oid'], FILTER_SANITIZE_URL);
$stmt = $link->prepare("SELECT `customer_id` FROM `order_summary` WHERE `order_id` = ?");
$stmt->bind_param("i", $oid);
$stmt->execute();
$result = $stmt->get_result();
if($result) {
$row = $result->fetch_assoc();
$id_check = sanitize($row['customer_id']);
}
if($_SESSION['customer_id'] === $id_check) {
$released = "Released";
$stmt = $link->prepare("SELECT `voucher_prefix`, `voucher_no` FROM `vouchers` WHERE `order_id` = ? AND `product_id` = ? AND `status` = ?");
$stmt->bind_param("iis", $oid, $pid, $released);
$stmt->execute();
$result = $stmt->get_result();
if($result) {
while($row = $result->fetch_assoc()) {
$voucher_code = sanitize($row['voucher_prefix']) . "#" . sanitize($row['voucher_no']);
echo $voucher_code . "<br/>";
}
}
} else {
header("location: dashboard.php");
}
} else {
header("location: dashboard.php");
}
} // end functionASKER CERTIFIED SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Yes, that logic sounds right to me.
You might also consider using POST instead of GET. This would prevent the user from being able to change what record they are seeing by changing the url. Or, use AJAX to "get" the data -- again, avoiding reliance on url parameters.
The POST vs GET argument basically boils down to this question: Do you mutate any data on the server? If the answer is "yes," you must use POST (or PUT or DELETE, as appropriate). If the answer is "no," you are free to use GET. There are some edge cases, but those are the center-of-gravity issues.
Why would you prefer to use GET? You would prefer GET if you want to be able to share a clickable link. You would prefer GET if you want to be able to bookmark the URL. You would prefer GET if you want to be able to test from a browser address bar. If things are correctly designed, anyone using a GET request will still go through client authentication before the script reveals the data. In my experience, the value of GET outweighs any perceived disadvantages.
Whether you use GET or POST, and whether you hide HTTP requests behind an AJAX facade, you still have to write the same authentication code into your PHP scripts. Otherwise, some day, someone will decide to try a direct POST request, and if your script does not contain the authentication code, the direct request will divulge the data you were hoping to protect.
Why would you prefer to use GET? You would prefer GET if you want to be able to share a clickable link. You would prefer GET if you want to be able to bookmark the URL. You would prefer GET if you want to be able to test from a browser address bar. If things are correctly designed, anyone using a GET request will still go through client authentication before the script reveals the data. In my experience, the value of GET outweighs any perceived disadvantages.
Whether you use GET or POST, and whether you hide HTTP requests behind an AJAX facade, you still have to write the same authentication code into your PHP scripts. Otherwise, some day, someone will decide to try a direct POST request, and if your script does not contain the authentication code, the direct request will divulge the data you were hoping to protect.
ASKER
I didn't use POST because the GET requests are not altering any data in the database. I use that as my simple rule of thumb (Hopefully I am correct in doing so). So, if I plan to alter the database in any way I use POST otherwise I use GET if I will only be displaying records from the database. In this case I am only displaying data but was concerned about users fiddling with the variables in the GET request. But, as per my code I think I have sorted out that concern.
On the subject of GET vs POST or AJAX:
There is still an extra layer of security by not exposing the record id in GET. The argument is not that you avoid authentication code (of course you still need it)... but displaying record id's in the url defies security through (some level of) obscurity. If there is no reason to prevent users from seeing data, then by all means, use GET. But when it comes to data that should be protected, why advertise your record ids? GET will potentially leave those id's in cache, or as a copy-able url. Record id's as url parameters are an invitation to potential nefarious activity on several levels.
There is still an extra layer of security by not exposing the record id in GET. The argument is not that you avoid authentication code (of course you still need it)... but displaying record id's in the url defies security through (some level of) obscurity. If there is no reason to prevent users from seeing data, then by all means, use GET. But when it comes to data that should be protected, why advertise your record ids? GET will potentially leave those id's in cache, or as a copy-able url. Record id's as url parameters are an invitation to potential nefarious activity on several levels.
ASKER
Thanks for your input Megan. I was quite happy to follow the rule I had set for myself regarding POST and GET but I understand your point.
Since I am not clued up on AJAX at this point in time, I would probably need to put the values of the ID's into hidden fields inside of a form and POST the form. Correct?
If this is the case, couldn't I as a user just view the page source or right-click the inspect button in chrome and go and manipulate the ID values that way? It is slightly more effort than changing the ID value in the url but it can be done I think.
I do this for testing my honey pot. I simply right-click, then click on inspect. I then change value="" to value"some value" and then I get the error message back saying that the form can't be submitted because I had validation telling the form only to submit if value of the hidden field is empty i.e.:""
So, what is to stop a user doing the same thing and going to the hidden value of the ID and changing it?
Again, I could be completely wrong, but I think that this could be done in which case it would be exactly the same as someone changing the ID value in the url.
Since I am not clued up on AJAX at this point in time, I would probably need to put the values of the ID's into hidden fields inside of a form and POST the form. Correct?
If this is the case, couldn't I as a user just view the page source or right-click the inspect button in chrome and go and manipulate the ID values that way? It is slightly more effort than changing the ID value in the url but it can be done I think.
I do this for testing my honey pot. I simply right-click, then click on inspect. I then change value="" to value"some value" and then I get the error message back saying that the form can't be submitted because I had validation telling the form only to submit if value of the hidden field is empty i.e.:""
So, what is to stop a user doing the same thing and going to the hidden value of the ID and changing it?
Again, I could be completely wrong, but I think that this could be done in which case it would be exactly the same as someone changing the ID value in the url.
The user could see the id's in the hidden fields if they inspected the HTML, but I'm pretty sure those changes don't get posted. Regardless of whether or not dev tools actually POST the changes, there is a way to "fake" POST, so whether it can be done through F12 is a moot point.
I didn't say hiding the id in POST is completely secure. But it is a layer of security. Someone would have to go through an extra hoop or two to do something nefarious. It's like closing a window to keep a burglar out. The burglar can find another way in if they're determined, but closing the window at least takes the obvious / visible invitation away.
You also avoid the issues with cached record id's and copy-able url links if the data is in POST. Again, closing the window on other vectors of possible attack.
I didn't say hiding the id in POST is completely secure. But it is a layer of security. Someone would have to go through an extra hoop or two to do something nefarious. It's like closing a window to keep a burglar out. The burglar can find another way in if they're determined, but closing the window at least takes the obvious / visible invitation away.
You also avoid the issues with cached record id's and copy-able url links if the data is in POST. Again, closing the window on other vectors of possible attack.
ASKER
As far as my testing went, If you have a submit button for the form and you manipulate the hidden form variables and then post the form by hitting submit, it will use the tampered with variables. But anyway, I agree with you. It would make it more difficult and more of a pain for people who don't know how to do it. It would be far easier to manipulate the URL and using POST would add an extra layer of security but still not fool proof which is why I added the extra measure of checking the user id session variable against the record in the database.
I suppose since either post or get leave you somewhat exposed, I might in future try out POST using that extra authentication layer.
Thanks for the idea, it's valuable feedback.
I suppose since either post or get leave you somewhat exposed, I might in future try out POST using that extra authentication layer.
Thanks for the idea, it's valuable feedback.
ASKER
I store the order ID in the voucher table as well. I am however not storing the user_id in the voucher table, only the order summary table.
voucher_id product_id order_id
101 12 51
102 27 51
So, in my query I am first checking that product id and order id are set. If they are, I get the customer id from the order summary table based on the order id.
If their current session user id matches that customer id from the query then it is the correct user and I can go ahead and display the voucher codes to them.
That is what I think I have coded anyway, haha!