Безопасна ли эта функция удаления PHP / MySQL? - PullRequest
Новая
       12

Безопасна ли эта функция удаления PHP / MySQL?

4 голосов
/ 21 мая 2011

У меня есть установка, в которой я удаляю записи из таблицы.

Он основан на строке запроса URL-адреса, который, я думаю, может быть плохим способом начать.

Так что, если URL-адрес:

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

И php в delete.php выглядит следующим образом: 

$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);

Есть ли способ сделать это более безопасным ... или это действительно каким-либо образом безопасно вообще?

EDIT:

ОК, поэтому, основываясь на отзывах, я выбрал новый подход.

list.php содержит набор радиокнопок для каждой записи в таблице: 

$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);

И delete.php выглядит так: 

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");

Это лучший способ сделать это?

Ответы [ 4 ]

11 голосов
/ 21 мая 2011
  1. У вас есть уязвимость, связанная с внедрением SQL, поскольку вы не очищаете параметры GET, введенные в запрос. Атакующий может использовать это, чтобы удалить все элементы в вашей таблице.
    Чистым решением для этого является использование prepared Statements.
    Быстрое и грязное решение - поставить их в кавычки и пропустить через mysql_real_escape_string.
  2. Даже если вы исправите эту часть, если злоумышленник сможет угадать действительную пару id / ref, он может удалить эту запись.
  3. Если параметр является целым числом, то почему бы вам не сделать его тип целочисленным? Что-то вроде $id=intval($_GET['id'])
5 голосов
/ 21 мая 2011

GET считается безопасным методом и не должен иметь побочных эффектов:

В частности, установлено, что методы GET и HEAD НЕ ДОЛЖНЫ иметьзначимость принятия действий, кроме поиска.Эти методы следует считать «безопасными».

В вашем случае ваш сценарий может быть уязвим для Подделка межсайтовых запросов .Лучше вместо этого использовать POST и рассмотреть какую-либо проверку подлинности и проверки авторизации перед удалением.

Кроме того, поскольку вы используете переданные параметры неаудированными и неизмененными, вы также уязвимы для SQL-инъекций .

2 голосов
/ 21 мая 2011

По крайней мере, вы должны поместить эти значения в параметры, а не вставлять их прямо в оператор SQL.Прямо сейчас вы уязвимы для атаки SQL-инъекций.Вот хорошая статья о том, как параметризовать ваш запрос, использовать хранимую процедуру или проверить входящий оператор.Это должно значительно помочь вашей безопасности:

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

1 голос
/ 21 мая 2011 
mysql_query(sprintf("DELETE FROM mytable WHERE id='%s' AND ref='%s'", mysql_real_escape_string($id),mysql_real_escape_string($res)));
...