Geek Answers Handbook

Is this feature safe for PHP / MySQL?

I have a setting where I delete records from a table.

It is based on a request for a URL, which I think may be unsuccessful to start with.

So if the url is:

http://www.example.com/delete.php?id=123&ref=abc

And the php in the delete.php file looks like this:

 $id=$_GET['id']; $ref=$_GET['ref']; $con = mysql_connect("blahblah","user","password"); if (!$con) { die('Could not connect: ' . mysql_error()); } mysql_select_db("test", $con); mysql_query("DELETE FROM mytable WHERE id=" . $id . " AND ref='" . $ref . "'"); mysql_close($con);

Is there a way to make this safer ... or is it not protected at all?

EDIT:

OK, so based on the feedback, I took a new approach.

list.php contains a set of radio objects for each entry in the table - as follows:

 $con = mysql_connect("localhost","username","password"); if (!$con) { die('Could not connect: ' . mysql_error()); } mysql_select_db("db", $con); $result = mysql_query("SELECT * FROM myTable"); echo "<form name='wer' id='wer' action='delete.php' method='post' >"; echo "<table border='1'>"; while($row = mysql_fetch_array($result)) { echo "<tr>"; echo "<td>" . $row['title'] . "</td>"; echo "<td><input type='radio' name='test1' value='" . $row['id'] . "' /></td>"; echo "</tr>"; } echo "</table>"; echo "<input type='submit' name='submit' value='Submit' />"; echo "</form>"; mysql_close($con);

And delete.php looks like this:

 function check_input($value) { if (get_magic_quotes_gpc()) { $value = stripslashes($value); } if (!is_numeric($value)) { $value = "'" . mysql_real_escape_string($value) . "'"; } return $value; } $con = mysql_connect("localhost","user","password"); if (!$con) { die('Could not connect: ' . mysql_error()); } $varID = check_input($_POST["id"]); mysql_select_db("db", $con); $sql="DELETE FROM myTable WHERE id IN (" . $varID . ")"; if (!mysql_query($sql,$con)) { die('Error: ' . mysql_error()); } mysql_close($con); header("Location: list.php");

Is this the best way to do this?

+4
source share
4 answers
  • You have a SQL injection vulnerability because you do not sanitize the GET parameters that you entered in your query. An attacker can use this to remove all items in your table.
    A clean solution for this is prepared Statements .
    A quick and dirty solution puts them in quotation marks and runs them through mysql_real_escape_string .
  • Even if you fix this part, if an attacker can guess the actual id / ref pair, he can delete this entry.
  • If the parameter is an integer, then why don't you make its integer too? Something like $id=intval($_GET['id'])
+11
source

GET is considered a safe method and should not have any side effects:

In particular, it was found that GETs and HEAD Methods SHOULD NOT have the value of taking action other than searching. These methods should be considered safe.

In your case, your script may be vulnerable to Cross-Site Request Forgery . You better use POST instead and consider authentication and authorization before uninstalling.

In addition, since you use the passed parameters unaudited and unmodified, you are also vulnerable to SQL Injections .

+5
source

At the very least, you should put these values ​​in parameters, and not embed them directly in your SQL statement. You are currently vulnerable to SQL Injection attacks. Here is a good article on how to parameterize your query, use a stored procedure, or test an incoming statement. This should greatly help your safety:

https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet

+2
source 
 mysql_query(sprintf("DELETE FROM mytable WHERE id='%s' AND ref='%s'", mysql_real_escape_string($id),mysql_real_escape_string($res)));
+1
source

More articles:


All Articles