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

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

Бесплатная лицензия 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

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

Популярные статьи по теме
Как различить C и C++ разработчиков по их коду

Дата: 12 Май 2022

Автор: Гость

Так уж случилось, что я пишу код для разных IoT-железок, связанных с электричеством, типа зарядных станций автомобилей. Поскольку аппаратных ресурсов, как правило, вполне достаточно, то основным фоку…
Отладочный вывод на микроконтроллерах: как Concepts и Ranges отправили мой printf на покой

Дата: 06 Май 2022

Автор: Гость

Здравствуйте! Меня зовут Александр, и я работаю программистом микроконтроллеров.
Нереальный baselining или доработки PVS-Studio для Unreal Engine проектов

Дата: 26 Апр 2022

Автор: Валерий Комаров

Статический анализатор PVS-Studio постоянно развивается: улучшаются различные механизмы, происходит интеграция с игровыми движками, IDE, CI/CD и другими системами и сервисами. Благодаря этому несколь…
Разбор некоторых вредных советов для С++ программиста

Дата: 21 Апр 2022

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

Юмор юмором, но осторожность не повредит. Вдруг кому-то не до конца понятно, почему какой-то из советов является вредным. Здесь можно найти соответствующие пояснения.
Четыре причины проверять, что вернула функция malloc

Дата: 20 Апр 2022

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

Некоторые разработчики пренебрежительно относятся к проверкам: удалось ли выделить память с помощью функции malloc или нет. Их логика проста – памяти всегда должно хватить. А если не хватит, всё равн…

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

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