Chromium: использование недостоверных данных

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

На самом деле, причиной уязвимости может стать ошибка почти любого типа, даже обыкновенная опечатка. Собственно, если найденная ошибка классифицируется согласно Common Weakness Enumeration, то значит она является потенциальной уязвимостью.

Анализатор PVS-Studio, начиная с версии 6.21, научился классифицировать найденные ошибки согласно Common Weakness Enumeration и назначать им соответствующий CWE ID. Возможно читатели уже обратили внимание, что в предыдущих статьях помимо номера предупреждения Vxxx я приводил ещё и CWE ID. Это значит, что рассмотренные ранее ошибки теоретически могут стать причиной уязвимости. Вероятность этого невелика, но она есть. Что интересно, нам удалось сопоставить тот или иной CWE ID почти с каждым предупреждением, выдаваемым PVS-Studio. Это значит, что сами того не планируя, мы создали анализатор, который способен выявлять большое количество weaknesses :).

Вывод. Анализатор PVS-Studio помогает заранее предотвращать многие виды уязвимостей. Публикация на эту тему: "Как PVS-Studio может помочь в поиске уязвимостей?".

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

Итак, давайте рассмотрим, какие дефекты безопасности я заметил в процессе разбора отчета, выданного PVS-Studio для проекта Chromium. Как я писал в вводной статье, отчёт я смотрел достаточно бегло, поэтому могут быть и другие, незамеченные мной ошибки. Задача статьи в общих чертах показать, как какие-то ошибки могут приводить к тому, что программа начинает обрабатывать некорректные или непроверенные данные. Я пока не определился, как лучше называть такие данные, и пока буду использовать термин «недостоверные данные».

Примеры ошибок

Предупреждение PVS-Studio: V519 CWE-563 The 'delete_result' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 381, 383. install_util.cc 383

Функция возвращает некорректный статус. В результате, другие части программы будут считать, что функция успешно удалила некое значение. Ошибка в том, что статус DELETE_FAILED всегда заменяется на статус DELETED.

Ошибку можно исправить, добавив ключевое слово else:

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

Библиотека PDFium (используется в Chromium).

CWE-570 There are identical sub-expressions 'that.BeginPos > EndPos' to the left and to the right of the '||' operator. cpvt_wordrange.h 46
• V501 CWE-570 There are identical sub-expressions 'that.EndPos < BeginPos' to the left and to the right of the '||' operator. cpvt_wordrange.h 46

Обратите внимание, что (E2 < B1) и (B1 > E2), это одно и то же. Аналогично (B2 > E1), это то же самое, что и (E1 < B2).

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

Теперь давайте рассмотрим большой и сложный фрагмент кода из библиотеки регулярных выражений RE2 (используется в Chromium). Если честно, я даже не понимаю, что здесь происходит, но в коде точно есть аномальная проверка.

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

А теперь функция с аномалией.

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

Если переменная c < 0x80, то функция прекратит свою работу. Если функция не завершит свою работу, а продолжит её, то можно точно сказать, что переменная c >= 0x80.

Теперь смотрим на условие:

Условие (c1 == c), отмеченное комментарием "// < cpp">#define VP9_LEVELS 14 extern const Vp9LevelSpec vp9_level_defs[VP9_LEVELS]; typedef enum < . LEVEL_MAX = 255 >VP9_LEVEL; static INLINE int log_tile_cols_from_picsize_level( uint32_t width, uint32_t height) < int i; const uint32_t pic_size = width * height; const uint32_t pic_breadth = VPXMAX(width, height); for (i = LEVEL_1; i < LEVEL_MAX; ++i) < if (vp9_level_defs[i].max_luma_picture_size >= pic_size && vp9_level_defs[i].max_luma_picture_breadth >= pic_breadth) < return get_msb(vp9_level_defs[i].max_col_tiles); >> return INT_MAX; >

CWE-119 Array overrun is possible. The value of 'i' index could reach 254. vp9_encoder.h 931
• V557 CWE-119 Array overrun is possible. The value of 'i' index could reach 254. vp9_encoder.h 932
• V557 CWE-119 Array overrun is possible. The value of 'i' index could reach 254. vp9_encoder.h 933

Хорошо, если этот код приведёт к Access Violation. Но на практике, скорее всего, начнут обрабатываться какие-то случайные данные, расположенные после массива vp9_level_defs.

Другая схожая ошибка использования данных за пределами массива встретилась мне в библиотеке SQLite (используется в Chromium).

Вначале обратите внимание, что массив yy_shift_ofst содержит в себе 455 элементов.

Также интерес для нас представляют два макроса:

Макрос YY_SHIFT_COUNT определяет максимальный индекс, который можно использовать для доступа к элементам массива yy_shift_ofst. Он равен не 455, а 454, так как нумерация элементов идёт с 0.

Макрос YY_MIN_REDUCE, равный 993, никакого отношения к размеру массива yy_shift_ofst не имеет.

Функция, содержащая слабую проверку:

Предупреждение PVS-Studio: V557 CWE-125 Array overrun is possible. The value of 'stateno' index could reach 992. sqlite3.c 138802

В функции сделана защита, что индекс при доступе к массиву не должен быть больше определённого значения. Из-за опечатки, или по другой причине, используется не та константа. Следовало использовать константу равную 454, а вместо этого значение индекса сравнивается с 993.

В результате, возможен доступ за границу массива и чтение произвольных недостоверных данных.

Примечание. Ниже есть правильный assert, но он не поможет в Release-версии.

Скорее всего, проверку надо переписать так:

Проект ICU (используется в Chromium).

Предупреждение PVS-Studio: V774 CWE-416 The 'mzMappings' pointer was used after the memory was released. zonemeta.cpp 713

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

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

В следующей функции проекта Chromium неправильно реализована защита от отрицательных значений.

Предупреждение PVS-Studio: V519 CWE-563 The '* offset' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 3543, 3544. ax_platform_node_win.cc 3544

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

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

Другие ошибки

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

Кстати, в статью можно было включить больше примеров ошибок, но я их уже «потратил» при написании предыдущих статей, а повторяться не хочется. Например, в статье «Chromium: опечатки», был вот такой фрагмент:

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

Также в этой статье я не стал рассматривать ситуации, когда недостоверные (испорченные) данные возникают из-за отсутствия проверки указателя, который возвращает функция malloc. Если функция malloc вернула NULL, это вовсе не значит, что возможна только ошибка разыменования нулевого указателя. Существуют и более коварные ситуации. Схематично они выглядят так:

Здесь не будет разыменования нулевого указателя. Здесь произойдёт запись данных непонятно куда и разрушение каких-то данных.

Это интересная история и я посвящу ей следующую отдельную статью.

Рекомендации

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

Так зачем тогда я вообще писал эту статью и выделил этот тип ошибок?

Чтобы вы знали о них. Знание о существовании какой-то проблемы уже само по себе помогает предотвратить её. Если кто-то не знает о какой-то проблеме, не значит, что её нет. Хорошей иллюстрацией будет вот эта картинка:

Что же можно всё-таки посоветовать:

1. Обновляйте используемые в вашем проекте библиотеки. В новых версиях могут быть исправлены различные ошибки, которые представляют собой уязвимости. Впрочем, надо признать, что уязвимость может появиться как раз в новой версии, а в старой отсутствовать. Но все равно, более хорошим решением будет обновлять библиотеки. Про старые уязвимости знает больше людей, чем про новые.
2. Тщательно проверяйте все входные данные, особенно поступающие откуда-то извне. Например, очень тщательно должны проверяться все данные, поступающие откуда-то по сети.
3. Используйте различные инструменты проверки кода. Например, проекту Chromium явно не хватает использования статического анализатора PVS-Studio :).
4. Объясняйте коллегам, что "простая ошибка при кодировании — не значит нестрашная ошибка". Если ваша команда разрабатывает ответственные приложения, то вы должны максимально сосредоточиться на качестве кода и уничтожать все, даже безобидные на вид ошибки.

Примечание про PVS-Studio

Как я уже сказал, анализатор PVS-Studio уже помогает предотвратить появление уязвимости, обнаруживая ошибки ещё на этапе написания кода. Но мы хотим большего и вскоре серьезно усовершенствуем PVS-Studio, введя в Data Flow анализ понятие «использование непроверенных данных».

Мы даже уже зарезервировали специальный номер для этой важной диагностики: V1010. Диагностика позволит выявлять ошибки, когда данные были получены из ненадёжного источника (например, присланы по сети) и используются без должной проверки. Отсутствие всех необходимых проверок входных данных часто является причиной обнаружения уязвимости в приложениях. Подробнее про это мы недавно писали в статье "PVS-Studio 2018: CWE, Java, RPG, macOS, Keil, IAR, MISRA" (см. раздел «Потенциальные уязвимости, CWE»).

Новая диагностика позволит существенно усилить анализатор в выявлении потенциальных уязвимостей. Скорее всего, диагностика V1010 будет соответствовать идентификатору CWE-20 (Improper Input Validation).

Заключение

Предлагаю вам и вашим коллегам почитать на нашем сайте статью "42 рекомендации". После неё программист не превратится в эксперта по безопасности, но узнает много нового и полезного. Особенно эти статьи будут полезны разработчикам, только недавно освоившим язык C или C++ и не подозревающим насколько глубока кроличья нора, в которую они попали.

Я планирую обновить «42 совета» и превратить их в «50 советов». Поэтому приглашаю подписываться на мой твиттер @Code_Analysis и наш RSS канал, чтобы не пропустить эту и другие интересные статьи в нашем блоге.

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Chromium: Use of Untrusted Data.