Этот PHP-код безопасен? - PullRequest
4 голосов
/ 25 февраля 2010

Я знаю, что должен использовать подготовленное утверждение, но мой следующий проект будет использовать подготовленное утверждение, мне просто нужно закончить это простое небольшое приложение.

Итак, мой вопрос:

Безопасен ли следующий фрагмент кода?

Я использовал htmlentities, а также mysql_real_escape_string, потому что я думал, что это безопасный вариант.

//Image 
$imageInput = $_POST['Image'];
$imageClean = htmlentities($imageInput, ENT_QUOTES, 'UTF-8');



//Inserts values into relevant field and creates a new row.
mysql_query("UPDATE ***** SET image='" . mysql_real_escape_string($imageClean) . "'     WHERE id=" . mysql_real_escape_string($idClean) . "");

чтобы добавить код для $ idClean:

//Id to change
if(ctype_digit($_POST['testimonial']))
{
    $idInput = $_POST['testmonial'];
    $idClean = htmlentities($idInput, ENT_QUTOES, 'UTF-8');
}

Спасибо за вашу помощь.

p.s, если бы вы могли предложить что-то добавить, что было бы здорово.

Ответы [ 6 ]

4 голосов
/ 25 февраля 2010

Зависит от того, насколько чист ваш $idClean.

WHERE id=" . mysql_real_escape_string($idClean) . "

mysql_real_escape_string только добавляет обратные слеши к \x00, \n, \r, \, ', " и \x1a, но не остановится злоумышленник, использующий

$idClean = "1 OR 1=1 AND POSSIBLY OTHER SQL STATEMENTS"

Вместо mysql_real_escape_string вы должны просто преобразовать его в int.

3 голосов
/ 25 февраля 2010

Вы должны применять экранирование сущности только в точке вывода - нет смысла экранировать данные до вставки базы данных. Тем не менее, вы делаете правильные вещи с точки зрения mysql_real_escape_string.

Кроме этого, как говорит @Piskvor, существует потенциальная проблема с переменной idClean. (Это приведение к int например?)

Вы можете использовать следующее, например:

mysql_real_escape_string(intval($idClean))
2 голосов
/ 25 февраля 2010

$ idClean откуда?

еще один $ _POST? это должно быть целое число, не так ли?

не делайте html-санацию, просто $idClean = (int)$_POST['id']; ... заставит его быть целым числом, "убив" все возможные инъекции xss / sql (только для $ idCelan, я имею в виду)

И вообще, нет единственного лучшего способа для очистки входных данных; все зависит от того, что ввод должен содержать , где он будет сохранен и как будет использоваться в будущем.

РЕДАКТИРОВАТЬ : после вашего комментария к ответу middaparka, я предполагаю, что $ idClean получен из формы (вероятно, скрытого ввода).

Если вы хотите, чтобы эта форма даже не использовалась maliciuos, я предлагаю вам добавить еще одно скрытое поле с хэшем $ idclean, а затем на странице процесса проверить хэш, чтобы увидеть, изменил ли кто-либо идентификатор вручную (если вы не делает это уже)

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

1 голос
/ 25 февраля 2010

Если предположить, что $ _POST ['Image'] является URI, нет. Кто-то может отправить URI с использованием схемы javascript: и заставить других людей запускать произвольный JavaScript.

0 голосов
/ 25 февраля 2010

Как уже было сказано, у вас есть проблема с id=xyz. Но вместо того, чтобы преобразовывать его в целое число в php, я бы также передал его как строковый литерал по двум причинам:

  • Вы можете изменить тип поля id, не касаясь этой конкретной части кода. id не всегда должно быть целым числом.
  • Диапазон значений (особенно максимальное значение) целого числа php может быть меньше, чем тип определения поля.

кодировка соединения влияет на поведение mysql_real \ escape_string (). Поэтому вы должны передать ресурс соединения в качестве второго параметра в mysql_real \ escape_string ().

Использовать ли htmlentities () или нет перед вставкой данных, зависит от того, чего вы хотите достичь. Если вам когда-нибудь понадобятся данные в формате, отличном от html / utf-8, вам придется преобразовать их обратно. Но так как это не совсем связано с безопасностью .... имейте свой путь; -)

$mysql = mysql_connect('..', '..', '..');
//...
$query = sprintf("
  UPDATE
    *****
  SET
    image='%s'
  WHERE
    id='%s'
  ", 
  mysql_real_escape_string($imageClean, $mysql),
  mysql_real_escape_string($id, $mysql)
);

Да, и кстати: Обязательный намек на подготовленные высказывания: http://docs.php.net/pdo.prepared-statements

0 голосов
/ 25 февраля 2010

Каков ожидаемый диапазон символов для «Изображения» и идентификатора? Хотя экранирование ввода - хорошая идея, вы всегда должны проверять, чтобы на входе содержались только допустимые символы для данного типа данных. например,

if (preg_match("/\w(\w|\\.){0,31}/", $imageInput) && preg_match("/\d{1,12}/", $idClean)) {
// do database update
} else {
// invalid input, let the user know!
}

Вышеупомянутое регулярное выражение для $ imageInput должно разрешать только имя изображения с буквенно-цифровым, подчеркиванием и точкой остановки в нем. Длина от 1 до 32 символов (вы хотите сопоставить это с определением вашей базы данных). Другие символы не допускаются. Я сделал регулярное выражение в памяти, простите, если выход не правильный, но принцип использования регулярных выражений для очистки от заведомо хорошего ввода - это путь.

Безопасность базы данных - не единственный аспект безопасности, который вам необходимо учитывать. Подумайте о межсайтовом скриптинге (XSS). Что, если пользователь вводит имя изображения как: javascript: alert ('abc'); затем, когда вы отобразите его обратно пользователю, оно будет выполнено в их браузере!

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...