views:

73

answers:

4
while($info = mysql_fetch_array( $data )) {
// Print out the contents of the entry
Print "<li><b>Name:</b> ".$info['name'] . " <br />";
Print "<b>ID:</b> ".$info['ID']." <br />";
Print "<b>Age:</b> ".$info['age'] ." <br />";
Print "<b>Location:</b> ".$info['location'] ."<br /> ";
Print "<form action=delete.php method=POST><input name=ID value="safe(.$info['ID']." type=hidden><input type=submit name=submit value=Remove class=submit></form></li>";
 }
Print "</ol>";

That's my code. I am, however, focusing on this line:

Print "<form action=delete.php method=POST><input name=ID value=".$info['ID']." type=hidden><input type=submit name=submit value=Remove class=submit></form></li>";

As has been pointed out in one of my previous posts, it's not safe against a SQL injection attack.

I've borrowed this function from another post:

function safe($value){
   return mysql_real_escape_string($value);
}

Now, how in the heck would I make this part safe from an attack?

value=".$info['ID']."

Your continued support is greatly appreciated.

+1  A: 

simply check whether the server has this entry or not.

To do this:

$query = 'SELECT * FROM `table` WHERE `ID` = '.(int)$_POST['ID'];

Casting it to integer should be quite safe. If it is a string, it will be converted as well. See PHP's Type Juggling: http://www.php.net/manual/en/language.types.type-juggling.php

Also, by checking whether this entry exists or not, you are double securing that the data is valid.

So:

$result=mysql_query('DELETE FROM `savannah` WHERE `ID`='.(int)$_POST['ID']);
if(!$result){
  die(mysql_error());
}
thephpdeveloper
Should I bother with mysql_real_escape_string? This thing I'm creating for my own education would rely using DELETE by finding the ID only. However, I'd like to secure strings I enter into the database, too. Any advice?
chris m
If it is already casted to integer, there's no need for mysql_real_escape_string (since it is already integer, there's no funny characters inside it!).
thephpdeveloper
Okay. The ID field type is int(1) - so I know it's an integer. This however doesn't protect me from people guessing IDs. So now I'm trying to implement this code you wrote, and I'm unsure of where to put it in delete.php. $result=mysql_query("DELETE FROM savannah WHERE ID='.(int)$_POST['ID']") or die(mysql_error()); gives me errors.
chris m
looks like you have extra quotes behind `$_POST['ID']`. i'll update the post to suit your request.
thephpdeveloper
Casting a string to int will return 0. http://www.php.net/manual/en/language.types.string.php#language.types.string.conversion
txyoji
@txyoji - isn't that a good thing if you are using a numeric ID?
thephpdeveloper
+2  A: 

YES YES YES YOU NEED ESCAPING (not to be too emphatic)

To protect this code from attack, you need to make sure that you have an authenticated user. Is the user that is viewing this page logged in? If so, where is their session stored?

Whatever logic you used to read the data you need to use when deleting the data.

// Read data in at top to prevent E_STRICT errors if user messes with Query string    
$User_ID  = $_SESSION['User_ID'];  //assuming it was authenticated 
$a_filter = (isset($_POST['a_filter']) ? $_POST['a_filter'] : '');

$query = "
    SELECT 
        a, b, c 
    FROM 
        table 
    WHERE 1
        AND a       = " . mysql_real_escape_string($a_filter) . "
        AND User_ID = " . intval($User_ID) . "
    ";

That query will ensure:

  1. That the user in question can only see their records.
  2. That it doesn't matter what hacking data they send, it will be escaped.

Now when you PRINT it to HTML, make sure you protect against XSS attacks:

<td><?= htmlspecialchars($data); ?></td>

or if you are in an attribute:

<input type="text" name="a_filter" value="<? htmlspecialchars($data, ENT_QUOTES); ?>" />

Now, when you delete the record, just make sure you apply the same safeguards...

// Read data in at top to prevent E_STRICT errors if user messes with Query string
$User_ID  = $_SESSION['User_ID'];  //assuming it was authenticated 
$ID       = (int) (isset($_POST['id']) ? $_POST['id'] : 0);

$query = "
    DELETE FROM 
        table 
    WHERE 1
        AND ID      = " . intval($ID) . " 
        AND User_ID = " . intval($User_ID) . "
    ";


This post covered escaping SQL, escaping HTML, escaping HTML attributes, and ensuring that the queries are authorized.

Here is a short post on where escaping is important:
http://blog.gahooa.com/answers/what-kind-of-data-needs-to-be-escaped/

gahooa
Wow. Thank you.
chris m
+1  A: 

the security here should probably be multi-level - there's not a lot else you can do on that specific field.

do you have a unique session to identify the user (even if you don't yet, it isn't too difficult to do this simply)?

if yes, then you can use that to determine if that user is allowed to delete items in general or even that item in particular.

that should protect against simple injection attacks...

as for escaping integer values - I generally cast the incoming value as an integer, and that typically removes the problem of rogue characters and invalid inputs:

$safe['value'] = (int) $_POST['value'];

If the string is not-numeric, your safe value will be 0.

Edit: I should mention that simply relying on the fact that the database field is an integer is not secure in anyway, and without escaping or casting the inputs before you create the query, you are opening yourself to SQL injection.

HorusKol
+1  A: 

PHP Filter (a php 5.2.x feature) allows you to both validate, and sanitize data.

http://us.php.net/manual/en/filter.examples.php

txyoji