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

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

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

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

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

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

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


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

>
>
>
31 февраля

31 февраля

05 Янв 2018

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

0550_February_31_ru/image1.png

Наша команда написала уже 5 статей (1, 2, 3, 4, 5), посвященных поиску ошибок в открытом проекте Chromium. И, видимо, скоро я напишу ещё несколько заметок в наш блог на эту тему.

Сейчас я изучаю отчёт, выданный анализатором PVS-Studio и просто выписываю найденные ошибки. Написание заметок - это уже следующий этап. Я люблю в начале просмотреть отчёт, а потом уже решаю, как и о чём лучше написать. Однако, одна ошибка мне особенно понравилась, и я решил описать её сразу, не откладывая на потом.

Ошибка живёт в библиотеке Protocol Buffers (protobuf), используемой в проекте Chromium. Protocol Buffers - это протокол сериализации (передачи) структурированных данных, предложенный Google как эффективная бинарная альтернатива текстовому формату XML.

Если бы я увидел эту ошибку пару месяцев назад, я бы прошел мимо неё. Ну ошибка и ошибка. Однако, увидев её, я сразу вспомнил эпический сбой кассовых аппаратов, который недавно произошел в России. 20 декабря крупнейшие ритейлеры и сети АЗС по всей России столкнулись со сбоями в работе новых кассовых аппаратов. Первыми о проблеме узнали жители Владивостока, далее она распространилась по стране по мере начала рабочего дня, затронув Новосибирск, Барнаул, Красноярск, Кемерово и другие крупные города.

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

static const int kDaysInMonth[13] = {
  0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
};

bool ValidateDateTime(const DateTime& time) {
  if (time.year < 1 || time.year > 9999 ||
      time.month < 1 || time.month > 12 ||
      time.day < 1 || time.day > 31 ||
      time.hour < 0 || time.hour > 23 ||
      time.minute < 0 || time.minute > 59 ||
      time.second < 0 || time.second > 59) {
    return false;
  }
  if (time.month == 2 && IsLeapYear(time.year)) {
    return time.month <= kDaysInMonth[time.month] + 1;
  } else {
    return time.month <= kDaysInMonth[time.month];
  }
}

Функция ValidateDateTime принимает на вход дату и должна определить, корректна она или нет. В начале выполняются основные проверки. Проверяется, что номер месяца лежит в диапазоне [1..12], дни находятся в диапазоне [1..31], минуты находятся в диапазоне [0..59] и так далее. В коде всё это хорошо видно, и не требует специальных пояснений.

Далее идёт более сложная проверка, есть ли определённый день в определённом месяце. Например, декабрь состоит из 31 дня, а в ноябре 31-ого число нет, так как в месяце только 30 дней.

Чтобы проверить дни и при этом не писать множество операторов if или длинный switch, используется вспомогательный массив kDaysInMonth. В массиве записано количество дней в различных месяцах. По номеру месяца из массива извлекается максимальное количество дней и используется для проверки.

Дополнительно учитывается, что год может быть високосным, и тогда в феврале на 1 день больше.

В общем функция написана просто и красиво. Вот только неправильно.

Из-за опечатки дни проверяются неправильно. Обратите внимание, что с максимально возможным количеством дней в месяце, сравнивается не день, а месяц.

Ещё раз взгляните на код:

if (time.month == 2 && IsLeapYear(time.year)) {
  return time.month <= kDaysInMonth[time.month] + 1;
} else {
  return time.month <= kDaysInMonth[time.month];
}

В сравнениях "time.month <=" надо использовать не член структуры month, а член day. Т.е. корректный код должен выглядеть так:

if (time.month == 2 && IsLeapYear(time.year)) {
  return time.day <= kDaysInMonth[time.month] + 1;
} else {
  return time.day <= kDaysInMonth[time.month];
}

Естественно номер месяца (от 1 до 12), всегда меньше количества дней в любом из месяцев.

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

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

Данная ошибка (вернее две ошибки) обнаруживается с помощью следующих предупреждений PVS-Studio:

  • V547 / CWE-571 Expression 'time.month <= kDaysInMonth[time.month] + 1' is always true. time.cc 83
  • V547 / CWE-571 Expression 'time.month <= kDaysInMonth[time.month]' is always true. time.cc 85

Как видите, теперь PVS-Studio также сразу классифицирует предупреждения согласно Common Weakness Enumeration (CWE).

Ещё хочу обратить внимание, что анализатор PVS-Studio всё более глубоко учится анализировать код. Сама по себе диагностика V547 старая (была реализована в 2010 году). Однако, скажем, год назад анализатор не смог бы найти эту ошибку. Теперь анализатор умеет заглянуть внутрь массива и понять, что извлекаются значения в диапазоне [28..31]. При этом он понимает, что 0 в массиве учитывать не надо, так как возможный диапазон time.month равен [1..12]. Если бы номер месяца был, скажем, равен 100, то произошел выход из функции и анализатор это понимает.

В итоге, анализатор видит, что происходят следующие сравнения диапазонов:

  • [2..2] <= [28..31]
  • [1..12] <= [29..32]

Следовательно, условия всегда истинны, о чём анализатор и выдаёт предупреждение. Вот такой вот глубокий анализ. Как видите, мы не только добавляем новые предупреждения в PVS-Studio, но и дорабатываем Data-Flow анализ, что сказывается на качестве уже существующих диагностик.

Почему первый диапазон [2, 2] представлен только одним числом 2? Дело в том, что учитывается уточняющее условие time.month == 2.

Следует задаться вопросом: как можно улучшить стиль, чтобы защититься от подобных ошибок?

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

Можно только порекомендовать писать более внимательно юнит-тесты и использовать профессиональные статические анализаторы кода, такие как PVS-Studio.

Спасибо за внимание. Пойду дальше изучать отчёт.

Update

Уже через час после публикации статьи ошибка была исправлена и были добавлены дополнительные тесты.

Популярные статьи по теме
Holy C++

Дата: 23 Ноя 2022

Автор: Гость

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

Дата: 01 Ноя 2022

Автор: Гость

Прочитав эту статью, вы узнаете следующее: способы, которыми можно продлить время жизни временного объекта в С++; рекомендации и подводные камни этого механизма, с которыми может столкнуться С++ прог…
Как мы баг в PVS-Studio искали или 278 Гигабайтов логов

Дата: 28 Окт 2022

Автор: Григорий Семенчев, Сергей Ларин, Филипп Хандельянц

Предлагаем вашему вниманию интересную историю о поиске бага внутри анализатора PVS-Studio. Да, мы тоже допускаем ошибки, но мы готовы засучить рукава и залезть в самую глубину "кроличьей норы".
0, 1, 2, Фредди забрал Blender

Дата: 26 Окт 2022

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

Эта статья могла бы получить название "Как PVS-Studio защищает от поспешных правок кода, пример N7". Однако так именовать статьи становится скучновато. Поэтому сейчас вы узнаете, причём здесь Фредди …
Примеры ошибок, которые может обнаружить PVS-Studio в коде LLVM 15.0

Дата: 25 Окт 2022

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

Компиляторы развиваются и выдают всё больше предупреждений. Остаются ли преимущества от использования статических анализаторов кода, таких как PVS-Studio? Да, так как анализаторы тоже развиваются. Пе…

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

Следующие комментарии
Unicorn with delicious cookie
Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо