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

11

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

class DivtoggleController extends Zend_Controller_Action {

    public function closeAction() {
        /* ... code removed for brevity ... */

        $req = $this->getRequest();
        $formData = $req->getPost();

        $d = $formData['div'];
        $i = $formData['id'];

        $dm = new Model_DivtoggleManager();
        $rs = $dm->setDivToggleById($d, $i);

    }

}


class Model_DivtoggleManager extends Zend_Db_Table {

    public function setDivToggleById($div, $id) {
        $result = $this->getAdapter()->query(
           "update div_toggle set " . $div . "=1 where id=" . $id
        );
    }

}

Итак, учитывая, что я удалил логику аутентификации / управления сеансом для краткости, кто может сказать мне, какая возможная проблема может быть с этим образцом?

Роджер Халлибертон
источник
Почему это не хранится в простом cookie?
Темная ночь
1
@Darknight, вы предполагаете, что есть сеанс для сохранения куки. Есть или нет, не имеет отношения к большей проблеме безопасности.
Роджер Халлибертон

Ответы:

24

Вы можете научить их. Каждый делает это в начале, даже вы. Если этот тип кода делает его в производство, это вина старших людей; не младший.

Редактировать:

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

Джон Крафт
источник
2
+1 Согласиться. Вы не можете винить (предположительно) младшего программиста, потому что вы приняли уровень знаний, которым они могут не обладать. (Тем не менее, вы хотели бы думать, что такие проблемы хорошо известны в наше время. С другой стороны, я хотел бы думать, что я был талантлив и хорошо выглядел и т. Д.) :-)
Джон Паркер
Достаточно справедливое утверждение. Думаю, мне следует задать дополнительный вопрос, спрашивающий, почему руководство будет настаивать на запуске в производство кода, который не был одобрен старшими программистами. Могу ли я рисковать своей работой из-за небольшой проблемы безопасности?
Роджер Халлибертон
1
@Roger Halliburton: Ну, если это проблема безопасности действительно каким - то образом получить эксплуатировали, что самое худшее , что могло случиться? И почему указание на проблемы безопасности стоило вам вашей работы? Разве руководство не хочет иметь дело с этим?
FrustratedWithFormsDesigner
1
@ Роджер - это только незначительно, пока тебя не взломают. :) Я думаю, что ввод кода в производство без проверки (независимо от уровня разработчика) является провалом. К сожалению, это очень распространенная практика во многих компаниях; и, как правило, для того, чтобы руководство начало ценить такие вещи, как проверки кода, обычно требуется много денег.
Джон Крафт
1
@ Роджер: Нужно пересмотреть весь код, а не только код, написанный «младшими» программистами. Даже старшие программисты делают ошибки.
Дин Хардинг
20

Взломайте их код перед их глазами, затем покажите им, как это исправить. Снова и снова, пока они не поймут.

Джо Филлипс
источник
4
Отправляйте им по электронной почте каждое утро: «Кстати, вчера вечером я снова удалил вашу базу данных с помощью еще одной уязвимости SQL-инъекции, которую вы оставили в своем коде. Я знаю, насколько вам нравится восстановление резервных копий базы данных!: D»
Ant,
2
@Ant: Будь милым. Сделайте это вскоре после запланированного резервного копирования, а не незадолго до этого.
Donal Fellows
@Donal: сделайте это вскоре после резервного копирования, затем скажите им, что резервное копирование не удалось, и дайте им потеть до конца дня, прежде чем восстанавливать резервную копию за ночь.
jwenting
8

Вы можете поручить им принять класс, как только они присоединятся к вашей компании, прежде чем они получат доступ к системе контроля версий, который знакомит их с инъекциями SQL, межсайтовым скриптингом, подделкой межсайтовых запросов и другими распространенными уязвимостями. Обсуждайте примеры лицом к лицу, разбивайте плохой код перед ними, заставляйте их взламывать плохой код и направляйте их на сайт OWASP для получения дополнительной информации, как только они «закончат обучение».

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

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

Знание - сила!

янь
источник
3
Лучше отсеять их до того, как они присоединятся к вашей компании. Если вы нанимаете кого-то для написания чего-либо, использующего SQL, не задавайте вопросов для интервью о внедрении, то это означает некомпетентность.
Вуббл
Я бы вообще согласился, но очень умный и амбициозный младший наемник намного, намного дешевле, чем «PHP-разработчик с N-летним опытом», который не гарантированно будет намного лучше. Умные младшие разработчики могут поднять этот материал очень быстро и быть весьма полезными.
Ян
3

Предполагая, что это небезопасность, на которую ссылались другие, как на разработчика любого уровня, легко забыть, что getPost () не защищает данные в первую очередь.

Один из способов обойти это:

  1. Напишите класс, который получает все данные POST / GET и записывает их как есть в класс Singleton, называемый «insecure_data». Затем очистите массивы POST / GET.
  2. Разработчикам, которые должны извлекать данные POST / GET из массива 'insecure_data', а не из массивов POST / GET.

Любой разработчик, который извлекает что-то из массива с именем «insecure_data» и не заботится о его защите, либо неосведомлен, либо ленив. Если это первое, проведите обучение, после чего оно должно быть последним, и тогда у вас возникнет дисциплинарная проблема, а не проблема программирования.

Дэн Блоуз
источник
Вау, это излишне запутано и все еще не решает проблему. Это не вопрос очистки входных данных просто для удовольствия, а очистки данных, которые вы помещаете в базу данных, и эти данные теоретически могут поступать откуда угодно, из веб-формы, электронной почты или XML-документа, который кто-то загружает. Во всех этих случаях вам нужно очистить, когда он идет в базу данных.
Роджер Халлибертон
@RogerHalliburton Конечно, не все решает, но это концепция (своего рода шаблон проектирования, если хотите), а не полностью продуманная мера безопасности, чтобы небезопасные данные выглядели небезопасными.
Дэн Блоу
Ах я вижу. Но вы просто боретесь с человеческой природой, если вы говорите кому-то: «Этот объект небезопасен, если он не помечен как защищенный», и они пытаются использовать его для чего-то, большую часть времени они будут отмечать его как безопасный без фактической проверки. Что ж, наслаждайтесь повышением репутации.
Роджер Халлибертон
С точки зрения младшего разработчика, просто просмотр $ insecure_data поднимает флаг так, как это делает просто просмотр $ postdata (или чего-то еще). Идея исходит от Джоэла Спольски «Делать неправильный код, выглядят неправильно» . Если человеческая природа ваших разработчиков такова, что они игнорируют этот красный флаг, то вам нужно либо провести большую подготовку, либо привлечь новых разработчиков.
Дэн Блоуз
Я не держу Спольского на пьедестале. Это разработчики, которые легко игнорируют несогласованные табуляции, они не собираются дважды думать об использовании чего-то, что называется «небезопасно».
Роджер Халлибертон
1

Одним из лучших руководств, которые я прочитал по веб-безопасности, является это руководство по безопасности Ruby on Rails . Хотя это Ruby on Rails, многие концепции применимы к любой веб-разработке. Я бы посоветовал любому новичку прочитать это руководство.

mistagrooves
источник
2
Все знают, что Руби не в безопасности.
Роджер Халлибертон
5
@ Роджер: «Все знают» все виды вещей, которые могут быть или не быть правдой. Я защищаю основанный на реальности подход к программированию, а не на основе слухов.
Donal Fellows
0

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

Райан
источник
1
о, я думал, что мы отвечаем на это как на тестовый вопрос:]
Райан
Прогнозы немного отстойны, так как формулировка вопроса была изменена ex post: P
Райан
0

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

Если они продолжают портить, несмотря на все вышеперечисленное (вы, возможно, захотите взглянуть на их коммиты и т. Д. Вместо того, чтобы узнать «вживую»), то проблема в том, что вы их не используете как наставника, или в том, что им, возможно, нужно найти что-то более подходящее, чтобы зарабатывать на жизнь.

Джон Паркер
источник