CODEWITHSUNDEEP
mysql
z0mn
z0mn

Reputation: 179

Making a form submit a variable safely

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.

Upvotes: 0

Views: 239

Answers (4)

txyoji
txyoji

Reputation: 6848

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

Upvotes: 1

HorusKol
HorusKol

Reputation: 8696

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.

Upvotes: 1

gahooa
gahooa

Reputation: 137252

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/

Upvotes: 2

mauris
mauris

Reputation: 43619

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());
}

Upvotes: 1

Related Questions