# 31 февраля

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

![0550_February_31_ru/image1.png](https://import.viva64.com/docx/blog/0550_February_31_ru/image1.png)

Наша команда написала уже 5 статей \([1](https://pvs-studio.ru/ru/blog/posts/cpp/a0074/), [2](https://pvs-studio.ru/ru/blog/posts/cpp/0113/), [3](https://pvs-studio.ru/ru/blog/posts/cpp/0205/), [4](https://pvs-studio.ru/ru/blog/posts/cpp/0225/), [5](https://pvs-studio.ru/ru/blog/posts/cpp/0442/)\), посвященных поиску ошибок в открытом проекте [Chromium](http://www.chromium.org/)\. И, видимо, скоро я напишу ещё несколько заметок в наш блог на эту тему\.

Сейчас я изучаю отчёт, выданный анализатором [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) и просто выписываю найденные ошибки\. Написание заметок \- это уже следующий этап\. Я люблю в начале просмотреть отчёт, а потом уже решаю, как и о чём лучше написать\. Однако, одна ошибка мне особенно понравилась, и я решил описать её сразу, не откладывая на потом\.

Ошибка живёт в библиотеке [Protocol Buffers](https://ru.wikipedia.org/wiki/Protocol_Buffers) \(protobuf\), используемой в проекте Chromium\. Protocol Buffers \- это протокол сериализации \(передачи\) структурированных данных, предложенный Google как эффективная бинарная альтернатива текстовому формату XML\.

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

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

```cpp
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 день больше\.

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

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

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

```cpp
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_\. Т\.е\. корректный код должен выглядеть так:

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v547/) / [CWE\-571](https://cwe.mitre.org/data/definitions/571.html) Expression 'time\.month <\= kDaysInMonth\[time\.month\] \+ 1' is always true\. time\.cc 83
* [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) / [CWE\-571](https://cwe.mitre.org/data/definitions/571.html) Expression 'time\.month <\= kDaysInMonth\[time\.month\]' is always true\. time\.cc 85

Как видите, теперь PVS\-Studio также сразу классифицирует предупреждения согласно Common Weakness Enumeration \([CWE](https://cwe.mitre.org/)\)\.

Ещё хочу обратить внимание, что анализатор 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**

Уже через час после публикации статьи ошибка была [исправлена](https://github.com/protocolbuffers/protobuf/pull/4147) и были добавлены дополнительные [тесты](https://github.com/protocolbuffers/protobuf/pull/4148)\.