Перейти к содержимому
BXWiki

Проверка кода перед релизом - чек-лист безопасности проекта

Проверяем свой код перед выкладкой по короткому списку: вывод и запросы, защита действий и прав, работа с файлами и секретами, опасные вызовы.

Механика

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

Список проверок делают бинарным: на каждый пункт отвечают «да» или «нет». Ответы вроде «вроде бы там всё нормально» означают, что пункт не проверен и его надо открыть и посмотреть.

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

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

Любое действие, меняющее данные, защищают проверкой сеанса и метода запроса. Снятие этих проверок ради удобства отладки - самая частая причина уязвимостей в своих контроллерах.

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

Секреты не живут в коде и не попадают в журналы. Ключи и токены хранят в настройках вне репозитория, а подписи сравнивают функцией постоянного времени.

Отладочные вызовы удаляют перед выкладкой полностью. Печать массивов и включённый показ ошибок на боевом сайте выдают структуру данных и пути к файлам первому же любопытному.

Шаги

  1. Собрать список изменённых файлов релиза и смотреть только их, а не весь проект.
  2. Проверить вывод: каждое значение из запроса или базы экранировано по контексту.
  3. Проверить запросы: нет склейки строк, ключи фильтра не приходят из запроса.
  4. Проверить действия: есть проверка сеанса, метода запроса и прав на операцию.
  5. Проверить файлы и секреты: белый список расширений, ключи вне кода и журналов.

Код

Собираем список изменённых файлов релиза:

Окно терминала
git diff --name-only origin/main...HEAD -- '*.php' | grep -v '^bitrix/'
# смотрим только свой изменённый код, а не весь проект целиком

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

Ищем незакрытый вывод в шаблонах:

Окно терминала
grep -rn '<?=\s*\$arResult' local/templates local/components | grep -v 'htmlspecialcharsbx' | head
# голый вывод значения в разметку - первый кандидат на проверку

Ищем склейку строк в запросах:

Окно терминала
grep -rn "query(.*\.\s*\$" local/ | head
grep -rn "'filter' => \$_REQUEST" local/ | head
# оба шаблона кода почти всегда означают проблему с безопасностью

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

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

Проверяем защиту своих действий:

public function configureActions(): array
{
    return ['save' => ['prefilters' => [
        new \Bitrix\Main\Engine\ActionFilter\Authentication(),
        new \Bitrix\Main\Engine\ActionFilter\HttpMethod([\Bitrix\Main\Engine\ActionFilter\HttpMethod::METHOD_POST]),
    ]]];   // снятые фильтры ищут отдельным поиском по проекту
}

Проверяем права на денежных операциях:

if (!$USER->IsAdmin() && (int)$order->getUserId() !== (int)$USER->GetID()) {
    throw new \RuntimeException('нет прав на этот заказ');   // проверка владельца
}
// авторизация отвечает «кто это», а права - «можно ли ему это делать»

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

Проверяем работу с загруженными файлами:

$file = $_FILES['photo'] ?? null;
if (!$file || !in_array(mb_strtolower(pathinfo($file['name'], PATHINFO_EXTENSION)),
        ['jpg', 'jpeg', 'png'], true)) {
    throw new \RuntimeException('недопустимый файл');   // белый список расширений
}
$fileId = \CFile::SaveFile(\CFile::MakeFileArray($file['tmp_name']), 'reviews');

Белый список расширений надёжнее чёрного: перечислить разрешённое проще, чем угадать все опасные варианты. Сохранение файла средствами платформы заодно раскладывает его по каталогам и чистит имя.

Ищем прямой вызов служебных файлов:

Окно терминала
grep -rLn "B_PROLOG_INCLUDED" local/php_interface local/lib --include='*.php' | head
# служебные файлы обязаны прерывать выполнение при прямом обращении из браузера

Прямое обращение к служебному файлу - способ выполнить кусок кода в обход проверок. Файлы, которые не являются точками входа, начинают со строки проверки и прерывания.

Ищем отладочные вызовы и секреты:

Окно терминала
grep -rnE 'var_dump|print_r\(|phpinfo|display_errors' local/ | head
grep -rn 'error_reporting(E_ALL)' local/ | head   # включённый показ ошибок в коде
grep -rnE "(token|secret|password)\s*=\s*'[^']{8,}'" local/ | head
# оба поиска обязаны возвращать пустой результат перед выкладкой

Ограничения

Список проверок не заменяет проактивную защиту и обновления платформы. Это второй эшелон: правильный код плюс включённый фильтр и свежие обновления работают только вместе.

Проверка глазами пропускает сложные логические дыры. Она ловит типовые шаблоны, а ошибки в бизнес-логике вроде неверного расчёта скидки находит только тестирование и разбор сценариев.

Автоматический поиск по шаблонам даёт ложные срабатывания. Каждую находку смотрят глазами: тот же самый вызов бывает и безопасным, если данные уже проверены выше по коду.

Проверку делает не автор кода, а кто-то другой из команды проекта. Свой код читается глазами автора, и его собственные ошибки в нём не видны: это подтверждается почти на каждом релизе.

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

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

Типичные проблемы

На странице выполняется чужой скрипт из отзыва покупателя.

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

Посетитель открывает чужой заказ подбором номера.

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

Действие выполняется по ссылке из чужого письма.

У изменяющего действия нет проверки сеанса и ограничения метода запроса. Обе проверки ставят на каждое действие, меняющее данные.

Ключ доступа к платёжной системе оказался в репозитории.

Секрет записан прямо в коде обработчика вместо настроек проекта. Ключи хранят в файле настроек вне репозитория и не пишут в журналы.

На боевом сайте видна печать массива с данными покупателей.

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

Частые вопросы

Сколько времени занимает такая проверка?

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

Кто должен проверять код?

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

Можно ли автоматизировать список?

Частично: поиск по шаблонам и статический анализ ловят заметную часть. Права, владельцы ресурсов и логика денег остаются за человеком.

Что делать с находками в чужом решении?

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

Нужна ли проверка для внутренних инструментов?

Да, и особенно там, где интерфейс кажется «только для своих». Внутренние страницы регулярно оказываются доступны снаружи по прямой ссылке.

Смежное

Первоисточник