Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

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

Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

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

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

31 Янв 2018

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

0557_Chromium_5_CWE_ru/image1.png

На самом деле, причиной уязвимости может стать ошибка почти любого типа, даже обыкновенная опечатка. Собственно, если найденная ошибка классифицируется согласно 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. Как я писал в вводной статье, отчёт я смотрел достаточно бегло, поэтому могут быть и другие, незамеченные мной ошибки. Задача статьи в общих чертах показать, как какие-то ошибки могут приводить к тому, что программа начинает обрабатывать некорректные или непроверенные данные. Я пока не определился, как лучше называть такие данные, и пока буду использовать термин "недостоверные данные".

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

Проект Chromium.

InstallUtil::ConditionalDeleteResult
InstallUtil::DeleteRegistryValueIf(....) {
  ....
  ConditionalDeleteResult delete_result = NOT_FOUND;
  ....
  if (....) {
    LONG result = key.DeleteValue(value_name);
    if (result != ERROR_SUCCESS) {
      ....
      delete_result = DELETE_FAILED;
    }
    delete_result = DELETED;
  }
  return delete_result;
}

Предупреждение 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:

if (result != ERROR_SUCCESS) {
  ....
  delete_result = DELETE_FAILED;
} else {
  delete_result = DELETED;
}

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

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

CPVT_WordRange Intersect(const CPVT_WordRange& that) const {
  if (that.EndPos < BeginPos || that.BeginPos > EndPos ||
      EndPos < that.BeginPos || BeginPos > that.EndPos) {
    return CPVT_WordRange();
  }
  return CPVT_WordRange(std::max(BeginPos, that.BeginPos),
                        std::min(EndPos, that.EndPos));
}

Предупреждения PVS-Studio:

  • V501 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

Условие написано неправильно. Чтобы легче было заметить ошибку, сократим условие:

if (E2 < B1 || B2 > E1 || E1 < B2 || B1 > E2)

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

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

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

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

typedef signed int Rune;
enum
{
  UTFmax = 4,
  Runesync = 0x80,
  Runeself = 0x80,
  Runeerror = 0xFFFD,
  Runemax = 0x10FFFF,
};

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

char*
utfrune(const char *s, Rune c)
{
  long c1;
  Rune r;
  int n;

  if(c < Runesync)    /* not part of utf sequence */
    return strchr((char*)s, c);

  for(;;) {
    c1 = *(unsigned char*)s;
    if(c1 < Runeself) {  /* one byte rune */
      if(c1 == 0)
        return 0;
      if(c1 == c)                // <=
        return (char*)s;
      s++;
      continue;
    }
    n = chartorune(&r, s);
    if(r == c)
      return (char*)s;
    s += n;
  }
  return 0;
}

Анализатор PVS-Studio выдаёт предупреждение на строчку, которую я отметил комментарием "// <=". Сообщение: V547 CWE-570 Expression 'c1 == c' is always false. rune.cc 247

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

if(c < Runesync)
  return strchr((char*)s, c);

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

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

if(c1 < Runeself)

Условие (c1 == c), отмеченное комментарием "// <=", выполняется только в том случае, если c1 < 0x80.

Итак, вот что мы знаем про значения переменных:

  • c >= 0x80
  • c1 < 0x80

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

Видеокодек LibVPX (используется в Chromium).

#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;
}

Предупреждения PVS-Studio:

  • V557 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

Массив vp9_level_defs состоит из 14 элементов. В цикле переменная i, используемая в качестве индекса массива, изменяется от 0 до 254. Итог: происходит выход за границу массива.

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

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

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

static const short yy_shift_ofst[] = {
  /*   0 */ 355, 888, 1021, 909, 1063, 1063, 1063, 1063, 20, -19,
  ....
  /* 450 */ 1440, 1443, 1538, 1542, 1562,
}

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

#define YY_SHIFT_COUNT    (454)
#define YY_MIN_REDUCE     993

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

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

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

static unsigned int yy_find_shift_action(....)
{
  int i;
  int stateno = pParser->yytos->stateno;

  if( stateno>=YY_MIN_REDUCE ) return stateno;      // <=

  assert( stateno <= YY_SHIFT_COUNT );

  do {
    i = yy_shift_ofst[stateno];                     // <=
  ....
}

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

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

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

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

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

if (stateno > YY_SHIFT_COUNT)
{
  assert(false);
  return stateno;
}

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

UVector*
ZoneMeta::createMetazoneMappings(const UnicodeString &tzid) {
  UVector *mzMappings = NULL;
  ....
  if (U_SUCCESS(status)) {
    ....
    if (U_SUCCESS(status)) {
      ....
      while (ures_hasNext(rb)) {
        ....
        if (mzMappings == NULL) {
          mzMappings = new UVector(
            deleteOlsonToMetaMappingEntry, NULL, status);
          if (U_FAILURE(status)) {
            delete mzMappings;
            uprv_free(entry);
            break;
          }
        }
        ....
      }
      ....
    }
  }
  ures_close(rb);
  return mzMappings;
}

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

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

if (U_FAILURE(status)) {
  delete mzMappings;
  mzMappings = nullptr;
  uprv_free(entry);
  break;
}

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

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

void AXPlatformNodeWin::HandleSpecialTextOffset(LONG* offset) {
  if (*offset == IA2_TEXT_OFFSET_LENGTH) {
    *offset = static_cast<LONG>(GetText().length());
  } else if (*offset == IA2_TEXT_OFFSET_CARET) {
    int selection_start, selection_end;
    GetSelectionOffsets(&selection_start, &selection_end);
    if (selection_end < 0)
      *offset = 0;
    *offset = static_cast<LONG>(selection_end);
  }
}

Предупреждение 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 записывается не туда, куда нужно. Правильный код должен быть таким:

if (selection_end < 0)
  selection_end = 0;
*offset = static_cast<LONG>(selection_end);

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

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

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

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

  if(!posX->hasDirtyContents() ||
     !posY->hasDirtyContents() ||
     !posZ->hasDirtyContents() ||
     !negX->hasDirtyContents() ||
     !negY->hasDirtyContents() ||          // <=
     !negY->hasDirtyContents())            // <=

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

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

int *ptr = (int *)malloc(100 * sizeof(int));
ptr[1234567] = 42;

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

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

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

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

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

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

0557_Chromium_5_CWE_ru/image2.png

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

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

Примечание про 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 канал, чтобы не пропустить эту и другие интересные статьи в нашем блоге.

Популярные статьи по теме
Эффект последней строки

Дата: 31 Май 2014

Автор: Андрей Карпов

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

Автор: Андрей Карпов

Краткое описание технологий, используемых в инструменте PVS-Studio, которые позволяют эффективно обнаруживать большое количество паттернов ошибок и потенциальных уязвимостей. Статья описывает реализа…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

Автор: Андрей Карпов

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

Дата: 14 Апр 2016

Автор: Андрей Карпов

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Дата: 22 Окт 2018

Автор: Андрей Карпов

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь ок…
PVS-Studio ROI

Дата: 30 Янв 2019

Автор: Андрей Карпов

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

Автор: Андрей Карпов

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

Автор: Андрей Карпов

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
Любите статический анализ кода!

Дата: 16 Окт 2017

Автор: Андрей Карпов

Я в шоке от возможностей статического анализа кода, хотя сам участвую в разработке инструмента PVS-Studio. На днях я был искренне удивлён тому, что анализатор оказался умнее и внимательнее меня.
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

Автор: Андрей Карпов

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…

Комментарии (0)

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