PillBoxCharger
PillBoxCharger

Reputation: 193

What is the best way to protect Mysql Delete statements?

I thought you guys would know the best way to do this:

When I delete an order ($prodId) from the ORDERS table, this script then goes and deletes all the items-ordered lines from the ORDERED_ITEMS table, which houses all the items ordered from every order in the system.

Is there a best practice to ensure that what I want deleted is deleted and only that? I'm worried about something going wrong/injected/mistyped with/into the script and accidentally deleting all the ordered item lines for all orders by mistake.

This is how far I got.

$delete_prod_items = mysqli_real_escape_string($con,$_REQUEST['prodId']);
if (is_numeric($delete_prod_items)){
    $sql3 = "DELETE from proteus.ordered_items where order_id = $orderId";
    mysqli_query($con,$sql3) or die('DELETE Order $orderId from the Ordered Items table failed: ' . mysqli_error($con).'<br>');     
}

Am I missing anything?

Upvotes: 0

Views: 83

Answers (4)

Vagabond
Vagabond

Reputation: 897

First things first you need to take care of the sql injections. The link will give you an idea.

Secondly you could use javascript to get a pop-up which asks the user a confirmation before deletion.

Next, to avoid the unintentional deletion of more than one row is to include LIMIT 1 to your query.

N.B. You could also limit priveleges by creating a different user (username) to access the mysql database use it in your mysql_connect('host', 'username', 'password', 'database') function . If you are displaying something really important you may consider not giving deletion rights.

Upvotes: 1

Phil
Phil

Reputation: 164910

As a first order of business, do the following...

  • Limit the request method to POST or even better, DELETE if you can get PHP to handle it.
  • Use prepared statements with bound parameters to avoid SQL injection vulnerabilities.

The main issue you will face is unauthorised requests. The best solution to this is to use a CSRF token.

Upvotes: 0

Rogue
Rogue

Reputation: 11483

Don't know why anyone is mentioning it, but the best way to really protect any statement is using PreparedStatements:

$delete_prod_items = mysqli_real_escape_string($con,$_REQUEST['prodId']);
$mysqli = new mysqli("localhost", "localuser", "password", "database");
if ($mysqli->connect_errno) {
    echo "Failed to connect to MySQL: " . $mysqli->connect_error;
}
if (!($stmt = $mysqli->prepare("DELETE FROM `proteus`.`ordered_items` WHERE `order_id` = ?"))) { //whatever query you want
    echo "Prepare failed: (" . $mysqli->errno . ") " . $mysqli->error;
}
$stmt->bind_param("s", $delete_prod_items);
$stmt->execute();
$stmt->close();

Also take after what @zerkms mentioned and use POST requests for your information.

Upvotes: 2

CMPS
CMPS

Reputation: 7769

  • Escaping data is good for preventing SQL Injection
  • It's better to limit the request with $_POST instead of $_REQUEST
  • If this page is for admin only, then it's fine since only you have access, however, if users have access to it, therefore it's better to apply some condition to the request before proceeding to the deletion (example, verify that they are deleting something related to their accounts only)

Upvotes: 0

Related Questions