CODEWITHSUNDEEP
phpmysql
sissi_luaty
sissi_luaty

Reputation: 2889

Mysql remove row - security issue

I have a shopping cart that is stored in a database. When the user deletes an item it should be removed from the database. The 1st approach that comes to my mind is to have a code such as:

<?php
    echo "<a href='./remove_items.php?id=1'>remove</a>";
?>

and the page "remove_items.php" would call a sql query to remove the item with the specified ID (eg.: 1), and after that go to the previous page with:

header("location: index.php");

the problem:
the page "remove_items.php" would be view-able in the source code, and therefore anyone could call it to hack my page with any item ID.

possible solution: checking if the owner of that item is the same user who made the query for deleting - is this okay? other suggestions?

Upvotes: 2

Views: 102

Answers (3)

Martin
Martin

Reputation: 22760

1) consider renaming your file, as if someone is nosing around your site as a shopper and see the name of the file, they know or can pretty easily work out what the file is doing. suggest renaming it to just something less obvious.

i.e.:

item_count.php

2) as mentioned by Mike Brent, GET is wide open to easy and simple abuses, but while his recommendation of POST is better, it's still open to abuse unless appropriate measures are taken (see 3 below).

3) SESSION and OBFUSCATION. Each shopping unit (cart, user) should have an associated SESSION for their browser data, this is not really to store the data itself but to prevent CSRF and similar activities spoofing one part of your website on another part of your website.

So, you can wrap the link to removing the item as a POST form with various hidden fields, one field should be a key value that is a unique reference generated each time the page is loaded, this reference is stored as a SESSION and then saved as a key, so then the recieving page (that deletes the product) can then match that the key in the form matches the key in the SESSION. This goes a long way towards preventing CSRF.

as well as this you want another hidden field to contain the id of the item to remove, of course.

...But the id should be not the product id but the row id of the row in the users shopping cart table row. So that when the page deletes the row it uses a cross referenced session id; so you take a POSTED value for the product,and then check that this value AND the session value for this cart (which is never shown to the user or their browser) match a row on the DB, if there's a match, then delete the row (or do any other action with it as needed).

Example:

Table products :

Product ID
product Name
...

Table Carts

Cart Id
Cart SESSION_id
Cart UserAgent *see below*

Table CartRow:

CartRow Id
Cart Id
Product Id
Quantity

So what we have is that the shopper buys something, it generates (if not already existing) a shopping Cart row, which contains their unique session data (never revealed to the browser), as well as a hash (password_hash for PHP,or similar) of their user agent concatenated with their IP [to fairly specifically identify their agent (although not perfect) ]. The useragent/IP hash is required to limit the slim possibility of session hijacking.

so, then the product is added to their CartRow with a reference to their Cart Id and the product Id, then when you submit the form to remove the product (or do any other basket action), you do a lookup request to check if the SESSION id and the useragent string hash matches and if they do get that cart id, then look in the CartRow table for the POSTed CartRow Id matching a row which also contains the Cart Id, once that row is found, bingo, you have authenticated the action so it's 99.999% safe to carry out and remove/add/edit their product.

Upvotes: 2

Sachin
Sachin

Reputation: 2775

Session in php
A session is a way to store information (in variables) to be used across multiple pages.A session creates a file in a temporary directory on the server where registered session variables and their values are stored. A session ends when the user loses the browser or after leaving the site, the server will terminate the session after a predetermined period of time, commonly 30 minutes duration.

My solution would be same for guest as well as registered one's , In case guest , a temporary id will be there , and a permanent one incase of a registered one.


  • Store user id in the session variable.
  • Check user authentication using that variable in the page for removing cart products.
  • if user is valid then only do the delete operation.
  • Since the session variable is available only in the server, using that for authentication is safe against hacking .

Upvotes: 1

Mike Brant
Mike Brant

Reputation: 71414

It is normally a REALLY bad idea to do any write, update, or delete operations via a GET call (i.e. parameter in query string). You have hit on the reason why.

I would suggest using POST along with appropriate form security measures, user authentication and authorization controls, etc.

Parameters in query string (GET requests) are typically useful for cases where you have a read use case and you want the end user to be able to directly bookmark the resulting view (i.e. view a product page with product id passed in query string or similar).

Upvotes: 4

Related Questions