Pull to refresh

Comments 57

Ну так, если всем надоела эта тема, то зачем писать?

нужно — например ...

нужно, например,…
Ошибки в личку пожалста, чтобы не засорять ненужным комментарии. А написал — дабы из кучи написанного сделать сухую выжимку и чтобы людям было легче все эти методы применять.
Добавлю про mysql_real_escape_string (и mysqli-шного аналога), мало кто знает, что вообще применение этой функции имеет смысл только для пары китайских кодировок (типа, GBK). Ну и кроме того, для корректной работы этой функции обязательно нужно использовать mysql_set_charset (иначе будет браться дефолтная кодировка), а выполнение SET NAMES не влияет на функцию экранирования.
использование PDO избавит от использования функций mysql_*
И причем тут это? Речь шла о понимании работы функций, PDO всего лишь обертка над всё теми же mysql_* функциями. Ну для примера, как Вы задаете кодировку соединения в PDO?
PDO это не обертка для «mysql_»-фнукций.
«mysql:host=localhost;dbname=dbname;charset=utf8»
Я имел ввиду, что PDO использует сишные функции mysql_*.
и хорошо умеет экранировать данные, что делает его намного безопаснее.
Ну мой вопрос про кодировки был к тому, что встречались такие вот коды
$pdo = new PDO(
    'mysql:host=hostname;dbname=defaultDbName',
    'username',
    'password',
    array(PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8")
); 

И естественно в таком виде функции экранирования правильно работать не будут.
Да и экранирует PDO всё теми же mysql_real_escape_string, так что PDO скорее позволяет не забыть что-то экранировать.
Числовые всегда привожу к числу (int)$var, (float)$var до вставки в запрос.

по поводу mysql_real_string_escape — это вы так думаете, что данные у вас обычные, а вот подсунут экзотическую, и что тогда?
В случае если кодировка стоит UTF / WIN экзотические данные не пройдут просто. Они даже могут ( если будут валидными ) — записаться, но инъекцию через них сделать будет нельзя.
А можно поинтересоваться? А статью прочли мою? PDO — это очень хорошая вещь — но это — далеко не панацея.
Если правильно использовать (preapared statements), то как тот огурец, избавляет от геморроя. Про воробьев и пушку не согласен, скорее, даже, наоборот.
Пушка и воробьи скорее здесь:
function getAsUint( &$var ) {
return ( preg_match( "~^\\s*(\\d+)\\s*$~i", $val, $t ) )? $t[1]: null;
}
А еще я бы добавил какой-нибудь strip_tags, тоже инъекция, то ли PHP, то ли SQL, то ли и то и другое.
Приведу лишь цитату :) «Это лишь пример, но суть понятна — Вы можете сделать свою супер функцию с блекджеком и феями нетяжелого поведения» ;)
Спорить можно буквально по каждому пункту.

1. Все данные из любых внешних источников [...] должны проходить проверку.

Ну проходить должны, да. Но проверять число ли это с помощью регулярных выражений… Точно, теперь у вас две проблемы. Привести к числу средствами языка и проще и быстрее. Но в любом случае — это самый правильный вывод во всей статье :)

2. В SQL запросе, который вы пишите, квотироваться должно ВСЕ, даже то, что вроде как и не нужно (например числовые значения)

А вот фиг! Мы про MySQL говорим, да? Иначе откуда в примере обратные кавычки… В MySQL — почитайте правила приведения типов — int и int сравниваются как int. Ну а int и строка — как числа с плавающей запятой. А это уже во-первых совсем другой результат (потеря точности для больших значений), а во вторых могут не работать оптимизации (потому что приведение типа это функция — CAST(id AS DOUBLE)

3. Все данные (а это не только то, что пришло извне но и то, что сформировалось внутри функции движка) должны проходить экранирование.

Ну да, то что сформировалось внутри функции движка. Вначале $a=1+2;, а потом $a надо экранировать? Это уже попахивает маразмом. Экранировать надо внешние данные, самой себе программа может доверять.

4.Использовать placeholders / prepared statements

Сам совет правильный, но дальше идет ерунда: «placeholders — это помощник, формирующий обычный текстовый запрос к БД [...] prepared statements — это способ попросить подготовить запрос базой данных». Все гораздо проще, placeholders — это те самые вопросительные знаки в тексте prepared statements вместо которых будут подставлены значения.

А где будут подставлены значение — на сервере (MySQL) или в клиенте (PHP) — это вопрос отдельный. Зависит от того, используете ли вы server-side prepared statements или client-side. Иногда у вас есть выбор и тогда безопаснее (но не лучше) выбирать server-side, иногда его нет.

И, кстати, про mysql_real_escape_string. Ей нужно знать в какой кодировке текст, чтобы правильно там все экранировать. Это важно для big5, cp932, gbk, sjis. Фишка тут в том, что для большинства (для всех остальных) кодировок можно тупо вставлять '\' перед каждым подозрительным символом. А в этих четырех можно подогнать два байта XX YY так, что
  • это дает некорректный символ в данной кодировке
  • то есть парсер увидит два символа XX (некорректный префикс) и YY
  • YY — подозрительный символ в ASCII и его надо экранировать
  • но когда вставляешь '\' — то есть получается XX 5C YY, то XX 5C — это корректный двухбайтовый код символа.
  • ну и по факту в строке XX 5C YY, этот YY ни фига не экранирован, так как это строка из двух символов — первый XX 5C, и второй YY.


Где-то так…
> Ну да, то что сформировалось внутри функции движка. Вначале $a=1+2;, а потом $a надо экранировать? Это уже попахивает маразмом. Экранировать надо внешние данные, самой себе программа может доверять.

Не совсем так. Экранировать нужно все строки, вне зависимости от их происхождения. Вообще все. Даже те, которые вы только что выбрали из базы и хотите неизменными положить обратно.
Совсем не так. Экранировать надо все внешние данные. Не только строки. А свои собственные сформированные данные — не надо. Даже строки. Если скрипт делает $a=''foobar'', то потом это экранировать бессмысленно.

Ну а строки, которые «только что выбрали из базы» — это из внешнего источника, так что надо экранировать, конечно. Не потому, что строки, а потому что из внешнего источника.
Окей.

$a = «foo'bar»;

Строка ваша собственная. Предлагаете не экранировать?
:)

Не предлагаю. Но это не имеет никакого отношения к SQL injection или к безопасности. Если я знаю, что в строка у меня, когда ее подставить в запрос, его может сломать — тогда я ее экранирую. А если там кавычка неэкранированная должна быть — тогда не экранирую (как в, например, $quote= $use_double? "\"": "'"; $sql=''select * from t1 where a=$quote$value$quote''; — тут как раз лучше $quote не экранировать, ага?).

Но по-любому, если это мой код, то экранирование не имеет никакого отношения к безопасности, и, соответственно, к теме статьи.
> Не предлагаю.

Нет, предложили. Вы сами сказали подразделять данные по источнику. В то время как я предлагаю подразделять их по типу. Мой вариант работает всегда безусловно, а в вашем варианте нужно всегда помнить что откуда пришло, и что где может быть.

> Но это не имеет никакого отношения к SQL injection или к безопасности.

Совершенно верно. Проблема экранирования НИКОГДА и не была проблемой безопасности как таковой. Это проблема составления синтаксически корректных запросов.

Золотое правило составления синтаксически корректных запросов — всегда экранировать все строки. Без всяких «мои данные», или «чужие данные», или «есть там кавычка» или «нет там кавычки».

> тут как раз лучше $quote не экранировать, ага?

Я не знаю какой usecase вы хотели изобразить. В отрыве от контекста код выглядит бессмысленным. Если $quote это исключительно кавычка — то добавьте её как есть в строковый литерал и проблема волшебным образом исчезнет

Остаюсь при своей точке зрения.
Не стоит упорствовать в глупости.
Любой может затупить или оказаться во власти старинного заблуждения. Это не позор.

Но настаивать на своём, когда тебе указывают на ошибку, да при том такую детскую — вот это уже стыдно.

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

И строки, которые вы хотите поместить в запрос, выбрав их только что из БД (а так же получив из любого другого источника, включая свой собственный код) необходимо форматировать не потому, что они какие-то «внешние», а потому что таков синтаксис строк в SQL: по краям должны стоять кавычки, а внутри проискейплены спецсимволы. Хочешь добавить в запрос — отформатируй по правилам.

И такая процедура должна быть единообразной.
Программист НЕ ДОЛЖЕН думать о том, какая у него строка или откуда она получена.
Если безопасность зависит внимательности от программиста, то это не безопасность.
Только автоматическое соблюдение правил гарантирует безопасность.

А автоматическое соблюдение правил в свою очередь гарантирует только использование плейсхолдеров. При котором форматирование данных производится скриптом, а не программистом.
Поэтому ваш пример ручной сборки запроса — чудовищен.
Вам должно быть стыдно демонстрировать такой код.
> А автоматическое соблюдение правил в свою очередь гарантирует только использование плейсхолдеров. При котором форматирование данных производится скриптом, а не программистом.
> Поэтому ваш пример ручной сборки запроса — чудовищен.

Эээ, а что делать писателям кода плейсхолдеров или других «автоматических правил»?
Писать реализацию плейсхолдеров и других «автоматических правил». Руками. Разве это не очевидно? (я, кстати, как раз сейчас именно об этом пишу в соседнем окне :)
Но разговор-то был не о них совсем.
И как руками писать эту реализацию? Примерно так и получится:

$quote= $use_double? "\"": "'";
replace($sql, '?', $quote$prepared_string_value$quote');
С этим твоим (псевдо)кодом я согласен.
С тем, который был приведён выше — нет. Там нет никакой сборки плейсхолдера, там сборка запроса. Это разные вещи.

Свет.
Я не очень понимаю контекст твоего замечания.
Если ты хочешь вступиться за коллегу, то его основная проблема — заявление

>«Не потому, что строки, а потому что из внешнего источника.»

И ты окажешь ему куда большую услугу, если объяснишь, что всё на самом деле РОВНО наоборот — «Не потому, что из внешнего источника, а потому что строки.»
Контекст моего замечания — это то, что задачи бывают разные. То есть кавычка в моём и его примере — тоже строка. Константа с названием поля тоже может передаваться в виде строки. То есть неважно как обозвать строки, которые нужно экранировать, но их нужно отличать от тех, которые ненужно.

Грубо говоря — если вопрос сузить для строк, подставляемых на место знака "?" в placeholder-е я согласна с тобой и zerkms. Оно проще в конце концов и экономия на escape немногих проверенных строк смысла не имеет.

Но если смотреть шире — я с вами не согласна. Задач существует явно больше, чем просто подставить значение на место знака "?". То же динамическое формирование запроса, а это может быть и в админке, и в ПО для администрирования баз. Та же реализация placeholder-ов в своём движке. Тут другие подходы работать будут. И это только банальное.
Нет-нет. Ты опять всё путаешь :)
Нет никаких «строк, которые экранировать не нужно». По определению строка — это то, что нужно экранировать. Всё остальное — не строка.
Кавычка в вашем примере — это ограничитель строки, а не сама строка.
Константа с названием поля — это не строка. Это идентификатор.
Понимаешь?

Задача у нас одна — составление безопасных SQL запросов.
Без разницы — в админке или в ПО.
И подходы везде одинаковые.

В частности, правила форматирования строк в Mysql такие:
— строка должна быть заключена в кавычки
— в ней должны быть экранированы спецсимволы
Эти правила, увы, никак не зависят от прикладной задачи :)

Я только не понял про «экономию на escape немногих проверенных строк». Ты точно поняла, о чём говорим мы с zerkms? Мы ничего экономить не предлагаем.
Мы предлагаем
1. Всегда соблюдать правила форматирования строк.
2. Любые данные всегда подставлять в запрос только через плейсхолдер.
Что из этого, по-твоему, неприменимо в каких-то частных случаях?
В общем, спасибо Свете, она мне показала точки зрения, с которых ваши слова не выглядят, как совсем ужасающий бред.
Но умоляю впредь быть точнее в формулировках.
Слово «экранирование» употреблять в узком смысле — «экранирование специсмволов в строковых литералах SQL», а не «форматирование произвольных частей SQL запроса, согласно правилам форматирования для „
Слово “строки» употреблять в узком смысле — «строковые литералы SQL»
по вопросу, форматировать ли элементы запроса, прописанные вручную в скрипте, придерживаться не варианта «всё разрешено, но есть исключения », а наоборот всё форматируем, могут быть исключения "? причём исключения эти ОЧЕНЬ нежелательны. поскольку, во-первых, прямая связь инициализации переменной с форматирвоанием её для помещения в запрос говорит о плохом коде, а во-вторых, вы вносите человеческий фактор в защиту от инхекций. В итоге один программист переменную создал, посчитал ненужной,
… отправка по ctrl-enter — зло.

Но, в общем, смысл понятен.
Слово «экранирование» просьба употреблять в узком смысле — «экранирование специсмволов в строковых литералах SQL», а не «форматирование произвольных частей SQL запроса, согласно правилам форматирования для них»
Слово «строки» просьба употреблять в узком смысле — «строковые литералы SQL», а не абстрактные строки на стороне клиента и не произвольные части SQL запроса.

И умоляю не писать ужасающих вещей типа «правила форматирования элементов запроса зависят от источника получения данных».

Вместо безусловного соблюдения чётких правил составления запросов (при этом инкапсулированных в библиотеку, а не разбросанных по коду) вводить человеческий фактор — отдавать на откуп программисту решение, форматировать ли переменную — это мина замедленного действия. Один написал переменную и не отформатировал. Другой взял, да и поменял, на такую, которую надо форматировать. Но он же не знал, что у первого в голове. И не отформатировал. Вылезла ошибка.

Вы, по-моему, так и не понимаете, что правила форматирования элементов запроса в ПЕРВУЮ очередь нужны для обеспечения синтаксической корректности SQL, а безопасность — это всего лишь побочный эффект.
И именно ваш подход (пусть девелопер решает, что форматировать, а что — нет) вместо жёсткого следования простому набору правил и приводит в конечном счёте к тому, что инъекции вообще до сих пор существуют.
экранирование такой строки может быть сделано в частном порядке: $a = «foo\\'bar»; либо $a = mysql_real_escape_string(«foo'bar»);
если предполагается ее использовать в качестве литеральной части запроса
> в частном порядке

Это бы работало, если бы люди никогда не ошибались, и если бы строки использовались только для бд. А то вам нужно её ещё и по почте отправить. Именно поэтому — экранирование нужно производить адресно, там и тогда, когда это нужно, а не заранее, когда ещё неизвестно, где данные могут быть использованы.

> либо $a = mysql_real_escape_string(«foo'bar»);

Либо можно не придумывать сложных правил и экранировать НЕПОСРЕДСТВЕННО ПЕРЕД ИСПОЛЬЗОВАНИЕМ, а не во время объявления. Следуя этому примитивному правилу у вас никогда не будет проблем.
принудительное экранирование внутренних данных может послужить причиной сокрытия серьезных логических ошибок
Нет никаких внутренних и внешних данных. Экранирование — инструмент для обеспечения синтаксической корректности SQL запросов. Ничего больше. Это только вопросы синтаксиса.

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

SELECT * FROM user WHERE user_id =

с пустым значением user_id, при принудительном обеспечении синтаксической целостности этот запрос пройдет без ошибок, иначе он не выполнится тем самым указывая на некорректность предварительной обработки данных.
> с пустым значением user_id, при принудительном обеспечении синтаксической целостности этот запрос пройдет без ошибок, иначе он не выполнится тем самым указывая на некорректность предварительной обработки данных.

А чем знание о некорректности нам может помочь? Лично мне всё равно какие данные передали — я просто приведу к целому и передам результат этого приведения. Мне абсолютно всё равно что было передано. Мои приложения от этого вообще никак не страдают. Разве только код становится проще.

> С точки зрения обработки ошибок некорректный запрос не очень отличается от нотиса при использовании неинициализированной переменной

Они очень отличаются. Впрочем — у меня в приложениях не бывает синтаксически некорректных запросов, потому что я следую паре простых правил при их написании.
Чувак. Ты всё перепутал :)))

Драйвер БД у тебя занимается валидацией данных, а форматирование их для помещения в запрос ты делаешь где-то в коде.
Так вот — всё должно быть наоборот! :)
Нет никакого «должно», есть только «может».

Безусловно изоляция слоев приложения помогает избежать многих ошибок, но эта же изоляция может затруднять решение задачи.

Например, у нас есть замечательная библиотека для работы с БД, она умеет автоматически собирать синтаксически правильные запросы, но при этом потребялет в качестве оплаты за свои услуги дополнительные ресурсы. Внезапно перед нами встает задача, которая загоняет библиотеку в мемори лимит. Что делать?

Если решение требует нетривиального использования БД, с конструированием сложных вложенных запросов основанных на критериях определяемых в разных местах кода, что делать?

Ручная сборка запросов — это как ассемблерные вставки в коде драйвера устройства, неудобно, опасно, но иначе не взлетит :(
Понятно.
Конкретных аргументов по теме обсуждения нет, есть только невнятные пассы руками.
Спасибо за ваше участие.
Просто в качестве совета на будущее
Даже при ручной сборке запросов также можно пользоваться плейсхолдерами, не колупаясь с форматированием вручную.

И, в любом случае, сборка синтаксически корректного SQL запроса никакого отношения не имеет к валидации данных. Это разные задачи. Рекомендую их не путать.

чего-чего?
каких-таких «внутренних данных»?
Ну поймите же уже — нет никакого «экранирования»
Нет никаких «внешних» или «внутренних» данных. Есть только данные, которые идут в SQL запрос. Самому запросу абсолютно всё равно, откуда данные поступали — правила форматирования одинаковы для всех!

Разбрасывать подготовку данных по коду — вот это действительно может послужить причиной серьезных логических ошибок. именно поэтому данные должны форматироваться перед самым попаданием в запрос, и нигде больше.

Именно поэтому мы не можем знать содержимое строки — и, следовательно, обязаны обрабатывать её так, как будто в ней есть спецсимволы
Разбрасывать или не разбрасывать подготовку данных по коду пределяется условиями и контекстом задачи, в этом вопросе нет решения идеального для любой ситауции.

Совершенно точно необходимо производить очистку данных на входе программы, но валидация и подготовка внутри может быть в любом удобном месте любым удобным способом.
Впервые встречаю человека, который не только не стыдится, но даже пропагандирует написание спагетти-кода.
Речь не об этом, а о том, что в этом споре нет единственно верного решения. Если вам нужно внести модификации в некачественный код — скорее всего придется поступиться своими принципами в угоду скорости разработки. Контекст и условия задачи имеют вес больший, чем стереотипы и привычные приемы.
Откуда в задаче появился некачественный код?

Мы вроде (ну как минимум я) говорили о лучших практиках. О том, как код писать правильно.
Лучшая практика — это та, которая лучше подходит к задаче. И мой комментарий был на обвинение в пропаганде «спагетти-кода».
Я только сейчас заметил ещё один классический косяк, от которого отвертеться будет посложнее.
И он, как раз, является прямым следствием представления об искейпинге, как о средстве защиты, в то время как это не более чем скромный форматтер строк:

> «Экранировать надо все внешние данные.»

В этом утверждении, на самом деле, сразу два заблуждения. Одно из них, про «внешние» отметил zerkms
Второе — «все».
Простой пример, который нам показывает, что искейпить «все внешние данные, а не только строки» в некоторых случаях чуть более, чем бессмысленно:

$limit = my_favorite_escaping_function($_GET);
$query = «SELECT * FROM table LIMIT $limit»;
Увы, накосячил с кодом. $_GET['limit'] разумеется, имелся в виду.
> Но проверять число ли это с помощью регулярных выражений… Точно, теперь у вас две проблемы. Привести к числу средствами языка и проще и быстрее.

Не для всех случаев верно. Например, PHP строку «sveta» приведёт к числу как 0. Если 0 — это допустимое значение, то будут записаны неверные данные.
Чувак. Даже после моих подробных ответов на вопросы, которые ты задавал мне в личку, у тебя не получилось :)

Ещё раз повторю одну очень важную вещь: писать статьи надо только на основании ПРАКТИЧЕСКОГО ОПЫТА, а не нахватавшись по-быстрому информации из статей и комментариев.
>> О том, как не допустить инъекций была уже масса статей — повторять не буду
И далее 3 экрана этого повторения :)
UFO just landed and posted this here
потому что вся нормальная разработка закрыла этот вопрос для себя ещё лет десять назад.

Я извиняюсь, а вы на дату статьи не забыли посмотреть? Примерно тогда она и была написана :)
Так что претензии по срокам немного мимо кассы.


А вот насчет вредности всё верно. Статья очень плохая и очень жаль, что она, судя по всему, всплывает в поиске.

UFO just landed and posted this here
Sign up to leave a comment.

Articles