Safely deleting a db record with php

Good Man wrote:

> MaXX <bs139412@skynet.be> wrote in
> news:e25ivo$17vf$1@talisker.lacave.net:
>
>> Good Man wrote:
>>> MaXX <bs139412@skynet.be> wrote in
>>> news:e258nf$pbt$1@talisker.lacave.net:
>>>> The problem is if I'm a nasty guy I just write my own form and
>>>> delete any record I want (since I'm auth'd) by just sending another
>>>> id.
>>> in your database, add a column called "keystring" and index it.
>>> populate it with 18 characters or so (write a PHP function that does
>>> this at the same time you enter the info in the database). So, this
>>> 'keystring' for record 1 might be '9jfhdsufs8ywre' while record 2
>>> might be 'agsadgiwqegiqw'.
>> It's the idea I have, but I need a to find a way to do this with an
>> absolutly unique "keystring" (md5*/sha1??) to avoid duplicate (may be
>> extremely rare, but this is the kind of bug you don't want to hunt one
>> day ;-) ...)
> to make a unique keystring, you could always md5 the current unix
> timestamp.
to be sure I've md5'd a concat of the timestamp (2005-11-12
19:11:14.043195+01) and the message and it seem to work at least with a few
hundreds of rows and I don't see how I can get any duplicates. Even if that
(unique constraint violation) the message will be logged again but a
slightly different timestamp...

Now I have a 32 chars unique identifier for each row, a bit too long but it
is doing the job fine.

[...]

Thanks again,
--
MaXX

>I want to delete a record from my db with a php script. Let's say I'm auth'd
>and I want to delete the record id 440. With a simple form (get or post), I
>send the id to my script and delete the record (DELETE FROM table WHERE
>id=some_validated_input).
>
>The problem is if I'm a nasty guy I just write my own form and delete any
>record I want (since I'm auth'd) by just sending another id.

Validate they have the authority to delete the record they want
to delete *AT THE TIME OF THE SUBMIT*.

>Is there any way to make arbitrary record deletion non-trivial in php? I'm
>thinking about a hash function to replace the real db id (DELETE FROM table
>WHERE record_hash=validated_form_hash),

You still need to validate that they have the authority to delete the
record *AT THE TIME OF THE SUBMIT*. The owner of the record may have
changed. The person deleting the record may have had the authority
to delete the record yesterday, but he was fired today, or his membership
expired.

>if possible without adding an
>awfull lot of server side computation.

Add a lot of server-side computation. You had to decide if he had the
authority to delete the record when the form was sent to the user. It can't
be that hard.

Gordon L. Burditt

Following on from MaXX's message. . .

I use a number of approaches
(1) Instead of auto-incrementing ID use a 32 bit random number.
(Obviously you have the creation overhead of making sure you can't
retrieve the record before creating it to catch. the theoretical 1 in
24billion chance of a clash)

This DOESNT solve your problem if any other IDs are available for
inspection. eg a selection table with click on button to delete
functionality because the other IDs can be harvested. AND WORSE if you
use this ID anywhere at all eg a table of click on button to _edit_
functionality the same thing applies.

However it WILL work for customer accounts where individual customers
never get to see a list of other customers. They can't then think let's
change ".../custdetails.php?custid=42" to "....?custid=43"

(2) Keep track of page visiting history in the session and boot out
people coming back via bookmarks without going through the right path.
Here's the outline:
In page 1
$_SESSION['LastPage']='Page1';
At top of page 2 :
if($_SESSION['LastPage']!='Page1'){...

(3) Wrap up each of your actions for each page in a handy object or
array and pop them into the session keyed by a random number. Use this
in your links
<a href="screenserver.php?A=2342542542">Delete record 1</a>
<a href="screenserver.php?A=71452726">Delete record 2</a>
Then in screenserver.php you do (in outline)
$a = $_GET[A];
$action = $_SESSION[actions[$a]];
$action = $_SESSION[actions] = array();
if($action->do=='del'}{....
if($action->do=='edit'}{....
$id = $action->id;


(4) If you want persistent links, for example you send a contact an
email with a "visit http:// ...?A=123435245" in it then do something
similar in a database table.


**** PS ****
I'm not doing much PHP at the minute. I've had the objects for (3) and
(4) working but never bothered to finish them off for public
consumption. If there's anyone who wants to finish them off then you
can probably work out the real email address if you feel like dropping
me a line.


--
PETER FOX Not the same since the cardboard box company folded
peterfox@eminent.demon.co.uk.not.this.bit.no.html
2 Tees Close, Witham, Essex.
Gravity beer in Essex <http://www.eminent.demon.co.uk>

>I use a number of approaches
>(1) Instead of auto-incrementing ID use a 32 bit random number.
>(Obviously you have the creation overhead of making sure you can't
>retrieve the record before creating it to catch. the theoretical 1 in
>24billion chance of a clash)
>
>This DOESNT solve your problem if any other IDs are available for
>inspection. eg a selection table with click on button to delete
>functionality because the other IDs can be harvested. AND WORSE if you
>use this ID anywhere at all eg a table of click on button to _edit_
>functionality the same thing applies.

All of this is a very poor substitute for validating that the user
in question has the authority to delete the record *AT THE TIME OF
THE FORM SUBMISSION*. If the user with administrator authority
always has the authority to delete *any* record, and a user without
administrator authority cannot delete any record (even his own),
there's nothing wrong with just using trivially-guessable record
numbers. But you need to re-check his administrator status at the
time of the form submission. He might have been fired between the
form being sent (and possibly cached in a browser for a year) and
submitting it.

If the user can only delete *his own* records, then check, when he
submits the form, that he still has the authority to delete it: he
still owns it, his membership hasn't expired, he's still logged in
as the same user, etc.

>However it WILL work for customer accounts where individual customers
>never get to see a list of other customers. They can't then think let's
>change ".../custdetails.php?custid=42" to "....?custid=43"

Someone malicious can still try running through all the numbers.

>(2) Keep track of page visiting history in the session and boot out
>people coming back via bookmarks without going through the right path.
>Here's the outline:
> In page 1
> $_SESSION['LastPage']='Page1';
> At top of page 2 :
> if($_SESSION['LastPage']!='Page1'){...

Unless you store which records the last page offered him the chance
to delete, and that it would still offer him the chance to delete
the record that he is trying to delete, this check is ineffective
and spoofable.

Gordon L. Burditt

MaXX wrote:
> Jiri Fogl wrote:
> > You should have more detailed authorization - not only auth'd
> > non-auth'd, but every user must have its access information, so system
> > can recognize who is that man who wants to delete.
> The problem in my particular case, is that the system can't know who will
> delete as there is no explicit ownership. The table in question is a log
> and the creator is a script.
>
> Your suggestion can be very usefull for another area of my project...
>
> Another idea is to only allow the php script to set a deleted flag wich only
> hide the record and wipe or undelete them by other means ...

The question isn't so much about ownership but authorization. If
deleting arbituary records in the table is not supposed to happen, then
logically it follows that there is a rule governing which records can
be deleted.

Oli Filth wrote:
>>>>> The problem is if I'm a nasty guy I just write my own form and
>>>>> delete any record I want (since I'm auth'd) by just sending
>>>>> another id.
>>>> in your database, add a column called "keystring" and index it.
>>>> populate
>>>> it with 18 characters or so (write a PHP function that does this
>>>> at the same time you enter the info in the database). So, this
>>>> 'keystring' for record 1 might be '9jfhdsufs8ywre' while record 2
>>>> might be 'agsadgiwqegiqw'.
>>> It's the idea I have, but I need a to find a way to do this with an
>>> absolutly unique "keystring" (md5*/sha1??) to avoid duplicate (may
>>> be extremely rare, but this is the kind of bug you don't want to
>>> hunt one day ;-) ...)
>> You could define the keystring column as a unique index. If on your
>> first insert you get back an error (implying a duplicate), then you
>> can just modify the keystring and insert again. Repeat until
>> success!
>>
>> Of course, if this is the method you go for, then using some sort of
>> hash is redundant; you might as well just generate random integers or
>> strings of a suitable length.
> Integers are probably better, because it will take less work for the
> DB
> to index them.

Instead of trying again and again to insert the record (although it would be
rare to have duplicates, depending on length), why not combine the an
autoincremented index & a key/hash/whatever? On deletion, both would have to
be given: duplicate key's are not a problem, because indexes are guarenteed
unique... Less time in script, but wether it's faster in de DB I have no
idea: you've got a nice small integer as unique number which can be found
very quickly, but you'd have to check 2 fields....

DELETE FROM table WHERE
id=some_validated_input AND
key=other_validated_input


Advantages are that every insertion works, "key"-length can be considarably
shorter (unless you're afraid for brute-force attacks..) and the unique
index/primary key has a logical value.

Grtz,

--
Rik Wasmus

Following on from Gordon Burditt's message. . .
>>I use a number of approaches
>>(1) Instead of auto-incrementing ID use a 32 bit random number.
>>(Obviously you have the creation overhead of making sure you can't
>>retrieve the record before creating it to catch. the theoretical 1 in
>>24billion chance of a clash)
>>
>>This DOESNT solve your problem if any other IDs are available for
>>inspection. eg a selection table with click on button to delete
>>functionality because the other IDs can be harvested. AND WORSE if you
>>use this ID anywhere at all eg a table of click on button to _edit_
>>functionality the same thing applies.
>
>All of this is a very poor substitute for validating that the user
>in question has the authority to delete the record *AT THE TIME OF
>THE FORM SUBMISSION*. If the user with administrator authority
>always has the authority to delete *any* record, and a user without
>administrator authority cannot delete any record (even his own),
>there's nothing wrong with just using trivially-guessable record
>numbers. But you need to re-check his administrator status at the
>time of the form submission. He might have been fired between the
>form being sent (and possibly cached in a browser for a year) and
>submitting it.
>
>If the user can only delete *his own* records, then check, when he
>submits the form, that he still has the authority to delete it: he
>still owns it, his membership hasn't expired, he's still logged in
>as the same user, etc.

Sorry Gordon, I should have made it clear that each page checks the user
as a matter of course. It didn't occur to me that some people don't do
this.


By the way : It _is_ a good idea to use big random unguessable numbers
for IDs because (a) it *obviously* makes the cracking job harder and (b)
even if you hit a valid number you have no idea whose it would be. Thus
it is a deterrent. Also (probably with more bits in the random number)
it is _essential_ where the user cannot be validated. For example
"Thank you for your custom...To view the progress of your order go to
www..../orders.php?OID=123454345434544"





>>(2) Keep track of page visiting history in the session and boot out
>>people coming back via bookmarks without going through the right path.
>>Here's the outline:
>> In page 1
>> $_SESSION['LastPage']='Page1';
>> At top of page 2 :
>> if($_SESSION['LastPage']!='Page1'){...

>Unless you store which records the last page offered him the chance
>to delete, and that it would still offer him the chance to delete
>the record that he is trying to delete, this check is ineffective
>and spoofable.


There are sometimes cases where persistent state in the database isn't
comprehensive enough. To take a simple example: Your delete page might
be preceded by a warning page about the dangers of deleting "Are you
sure" etc. In which case you code :
$_SESSION['DeleteWarningDone']=1;
in the warning page then at the top of the delete page
if($_SESSION['DeleteWarningDone']==1){...
(But why bother with specific code like this when you have standard page
tracking functions.)

Instead of delete, think of add (or submit order). Checking somebody's
authority is fine, but you need an idempotent method so that duplicate
orders aren't created by mistake.

--
PETER FOX Not the same since the submarine business went under
peterfox@eminent.demon.co.uk.not.this.bit.no.html
2 Tees Close, Witham, Essex.
Gravity beer in Essex <http://www.eminent.demon.co.uk>

>>All of this is a very poor substitute for validating that the user
>>in question has the authority to delete the record *AT THE TIME OF
>>THE FORM SUBMISSION*. If the user with administrator authority
>>always has the authority to delete *any* record, and a user without
>>administrator authority cannot delete any record (even his own),
>>there's nothing wrong with just using trivially-guessable record
>>numbers. But you need to re-check his administrator status at the
>>time of the form submission. He might have been fired between the
>>form being sent (and possibly cached in a browser for a year) and
>>submitting it.
>>
>>If the user can only delete *his own* records, then check, when he
>>submits the form, that he still has the authority to delete it: he
>>still owns it, his membership hasn't expired, he's still logged in
>>as the same user, etc.
>
>Sorry Gordon, I should have made it clear that each page checks the user
>as a matter of course. It didn't occur to me that some people don't do
>this.

If you check the user at a matter of course, then you can LET THE
USER SPOOF ALL HE WANTS. And random numbers are pointless in this
situation. If the user is properly logged in (which you check),
and he spoofs, then either he has the authority to delete the record,
which you should allow, or he doesn't, which you'll reject anyway.
If he's not logged in or doesn't have the authority to delete
records, he can spoof *ALL* of the numbers and still won't do any
deletions.

>By the way : It _is_ a good idea to use big random unguessable numbers
>for IDs because (a) it *obviously* makes the cracking job harder and (b)

If a user can only delete records he has the authority to delete anyway,
cracking attempts are pointless, so why bother preventing it?

>even if you hit a valid number you have no idea whose it would be. Thus

If the idea is to inflict random damage, it doesn't matter.

>it is a deterrent. Also (probably with more bits in the random number)
>it is _essential_ where the user cannot be validated. For example
>"Thank you for your custom...To view the progress of your order go to
>www..../orders.php?OID=123454345434544"

I don't think I'd feel comfortable implementing such a thing (if
it didn't require a login) if real money was involved. I'd worry
about putting any confidential information (e.g. an order) in
such a system also.

Gordon L. Burditt

Following on from Gordon Burditt's message. . .
>>it is a deterrent. Also (probably with more bits in the random number)
>>it is _essential_ where the user cannot be validated. For example
>>"Thank you for your custom...To view the progress of your order go to
>>www..../orders.php?OID=123454345434544"
>
>I don't think I'd feel comfortable implementing such a thing (if
>it didn't require a login) if real money was involved. I'd worry
>about putting any confidential information (e.g. an order) in
>such a system also.

Why?
.... IOD=123434343443 is a shared secret no different to a username and
password. The 'must login' approach is (a) cumbersome for the user, (b)
cumbersome for the sysadmin and (c) doesn't give any more security.



--
PETER FOX Not the same since the bridge building business collapsed
peterfox@eminent.demon.co.uk.not.this.bit.no.html
2 Tees Close, Witham, Essex.
Gravity beer in Essex <http://www.eminent.demon.co.uk>

>>>it is a deterrent. Also (probably with more bits in the random number)
>>>it is _essential_ where the user cannot be validated. For example
>>>"Thank you for your custom...To view the progress of your order go to
>>>www..../orders.php?OID=123454345434544"
>>
>>I don't think I'd feel comfortable implementing such a thing (if
>>it didn't require a login) if real money was involved. I'd worry
>>about putting any confidential information (e.g. an order) in
>>such a system also.
>
>Why?
>... IOD=123434343443 is a shared secret no different to a username and
>password. The 'must login' approach is (a) cumbersome for the user, (b)
>cumbersome for the sysadmin and (c) doesn't give any more security.

Because the ENTIRE shared secret needed for access is sent in a
single email. It is also likely that it will be recorded in browser
history (unlike web logins, where the logout procedure advises the
user to close the browser if it's a public system to get rid of
session cookies). Some browsers manage to leak browser history to
rogue sites using Javascript or Java. Ever notice how physical
credit cards and PINs are sent in DIFFERENT postal mails, usually
several days apart? There's a reason for that.

Yes, there is a difference in security.

Gordon L. Burditt