В этой статье я расскажу, как использовать PVS-Studio для статического анализа программного кода на языках С/C++ на примере open-source проекта Wireshark. Начну я с краткого описания анализатора сетевого трафика Wireshark и продукта PVS-Studio. Опишу подводные камни процесса сборки и подготовки проекта к статическому анализу. Постараюсь сформировать общую картину о продукте PVS-Studio, его преимуществах и удобстве использования, приводя предупреждения анализатора, примеры кода и собственные комментарии.

Анализатор сетевого трафика Wireshark


Для демонстрации возможностей PVS-Studio, требовалось найти известный, полезный и интересный open-source проект, анализ которого еще никто не делал. Я остановился на Wireshark, т.к. сам к нему не равнодушен, а если вы еще о нем не знаете, то возможно после прочтения этой статьи разделите мои чувства к нему.

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

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

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

Wireshark — кроссплатформенный инструмент, распространяемый под свободной лицензией GNU GPL, предназначенный для работы в Windows и Linux. Для формирования графического интерфейса используются библиотеки GTK+ и Qt.

Документацию и исходники программы можно найти на сайте.

Статический анализатор кода PVS-Studio


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

PVS-Studio — это статический анализатор C/C++/C++11 кода, поддерживающий компиляторы MS Visual С++, GNU GCC (MinGW), Clang, Borland C++.

PVS-Studio содержит следующие диагностики:
  • диагностика общего назначения;
  • диагностика 64-битных ошибок;
  • диагностика возможных оптимизаций.
Дополнительную информацию о PVS-Studio можно найти на сайте.

Сборка проекта Wireshark


Для проведения статического анализа скачаем исходники самой последней стабильной версии Wireshark 1.12.4. Сборку будем проводить в операционной системе Windows 7 для платформы Win64 с использованием компилятора, входящего в Visual Studio 2013. Дополнительно установим библиотеки Qt SDK 5.4.1 и WinPcap 4.1.3.

Сборку будем осуществлять из командной строки с использованием nmake. Для правильной работы сборочных скриптов установим Cygwin и Python 2.7.9.

Дополнительную информацию о сборке можно найти на сайте.

Несмотря на то, что сборка проводилась в полном соответствии с инструкцией, возник ряд ошибок. Для их устранения потребовалось:
  • Прописать путь до Cygwin в переменной окружения PATH, чтоб сделать доступной из консоли командную оболочку bash.
  • Отключить ACL управление доступом для NTFS в Cygwin, чтоб предоставить владельцу права на запись, чтение и запуск файлов.
  • Установить дополнительный пакет dos2unix в Cygwin. Т.к. для компиляции требовалась утилита u2d.
  • Потребовалось cкопировать файл Makefile.nmake из «asn1\hnbap» в «asn1\kerberos», чтоб заработала команда очистки «clean» для nmake.

Проведение статического анализа средствами PVS-Studio


У меня установлена PVS-Studio 5.25 с лицензией, но для первоначального знакомства с программой можно скачать и установить демонстрационную версию.

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

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

Для мониторинга запустим «Пуск\PVS-Studio\PVS-Studio Standalone», выберем пункт меню «Tools\Analyze Your Files ...» и нажмем кнопку «Start Monitoring». Далее из командной строки запустим сборку проекта «nmake -f Makefile.nmake all», как описано выше. Проверим, что сборка прошла успешно и завершим мониторинг, нажав на кнопку «Stop monitoring».

Запасемся терпением, т.к. далее автоматически запустится статический анализ. После его завершения сохраним plog-файл отчета, чтоб не запускать сборку и статический анализ несколько раз.

Уже на этом этапе в программе PVS-Studio Standalone можно приступить к поиску ошибок. Но чтоб использовать продвинутые возможности навигации по коду IntelliSense, я рекомендую открыть наш отчет в Microsoft Visual Studio.

Для этого выполним ряд действий:
  1. Создадим пустой проект Visual С++ в папке исходников Wireshark.
  2. В Solution Explorer перейдем в режим отображения файлов.
  3. Добавим исходники в наш проект.
  4. Откроем plog-файл отчета используя плагин: «PVS-Studio\Open Analysis Report».
Ну вот мы и подошли к самому интересному — поиску ошибок.

Поиск ошибок в проекте Wireshark


Приступим к поиску ошибок, просматривая предупреждения PVS-Studio и используя навигацию IntelliSense.

С самого начала меня привлекли комментарии в коде:
void decode_ex_CosNaming_NamingContext_NotFound(....)
{
  ....
  (void)item; /* Avoid coverity param_set_but_unused 
                 parse warning */
  ....
  /* coverity[returned_pointer] */
  item = proto_tree_add_uint(....);
  ....
}

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

Чтобы сформировать общую картину возможностей PVS-Studio будем искать ошибки разных типов, которые тяжело обнаружить из-за неопределенного поведения во время тестирования, требующие продвинутых знаний языка C/C++ и просто интересные. Для этого нам будет достаточно предупреждений первого и беглого просмотра предупреждений второго уровня.

Код:
typedef struct AIRPDCAP_SEC_ASSOCIATION {
  ....
  AIRPDCAP_KEY_ITEM *key;
  ....
}; 

void AirPDcapWepMng(....,AIRPDCAP_KEY_ITEM* key, 
  AIRPDCAP_SEC_ASSOCIATION *sa, ....)
{
  ....
  memcpy(key, &sa->key, sizeof(AIRPDCAP_KEY_ITEM));
  ....
}

Предупреждение: V512 A call of the 'memcpy' function will lead to the '& sa->key' buffer becoming out of range. airpdcap.c 1192

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

Для заполнения структуры 'AIRPDCAP_KEY_ITEM', находящейся по адресу 'key', используется не адрес 'sa->key' на такую же структуру, а адрес указателя на нее. Чтобы исправить эту ошибку, достаточно убрать лишнюю операцию получения адреса '&'.

Код:
typedef struct _h323_calls_info {
  e_guid_t *guid;
  ....
} h323_calls_info_t;

static const e_guid_t guid_allzero = {0, 0, 0, 
  { 0, 0, 0, 0, 0, 0, 0, 0 } };

void q931_calls_packet(....)
{
  h323_calls_info_t *tmp2_h323info;
  ....
  memcmp(&tmp2_h323info->guid, &guid_allzero, 16) == 0;
  ....
}

Предупреждение: V512 A call of the 'memcmp' function will lead to overflow of the buffer '& tmp2_h323info->guid'. voip_calls.c 1570

Еще один пример с неправильным использованием буфера. В одном из аргументов функции 'memcmp()' передается указатель на указатель на структуру 'e_guid_t', вместо указателя на нее.

Код:
#define ETHERCAT_MBOX_HEADER_LEN ((int) sizeof(ETHERCAT_MBOX_HEADER))

void dissect_ecat_datagram(....)
{
  if (len >= sizeof(ETHERCAT_MBOX_HEADER_LEN) && ....)
  {
    ....
  }
}

Предупреждение: V568 It's odd that the argument of sizeof() operator is the '(int) sizeof (ETHERCAT_MBOX_HEADER)' expression. packet-ethercat-datagram.c 519

При работе с памятью в C++ используется оператор 'sizeof()', который возвращает размер объекта или буфера в байтах. В нашем случае 'sizeof()' вернет в байтах размер типа 'int', вместо размера структуры 'ETHERCAT_MBOX_HEADER'. Для исправления ошибки надо убрать лишнюю операцию 'sizeof()'.

Код:
void Proto_new(....) {
  ....
  if (!name[0] || !desc[0])
    luaL_argerror(L,WSLUA_ARG_Proto_new_NAME,
      "must not be an empty string");
  ....
  if ( name ) {
    ....
    loname_a = g_ascii_strdown(name, -1);
    ....
  }
  ....
}

Предупреждение: V595 The 'name' pointer was utilized before it was verified against nullptr. Check lines: 1499, 1502. wslua_proto.c 1499

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

Проверка указателя 'name' проводится после использования 'name[0]'. С одной стороны, эта проверка избыточна, если указатель не нулевой, а с другой стороны, если он нулевой, все равно возникнет ошибка.

Код:
void create_byte_graph(....)
{
  ....
  u_data->assoc=(sctp_assoc_info_t*)g_malloc(
    sizeof(sctp_assoc_info_t));
  u_data->assoc=userdata->assoc;
  ....
}

Предупреждение: V519 The 'u_data->assoc' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1526, 1527. sctp_byte_graph_dlg.c 1527

В языках C/С++ выделение и освобождение памяти выполняется вручную. Ошибки освобождения выделенной памяти могут приводить к утечкам памяти.

Функция 'g_malloc()' выделяет участок динамической памяти размером 'sizeof(sctp_assoc_info_t)' байт и возвращает указатель на него. Но после изменения значения переменной, хранящей этот указатель, мы не сможем ни получить доступ к этому участку, ни освободить его, что приведет к утечке памяти.

Код:
PacketList::PacketList(QWidget *parent)
{
  QMenu *submenu;
  ....
  submenu = new QMenu(tr("Colorize with Filter"));
  /*ctx_menu_.addMenu(submenu);*/
  submenu = new QMenu(tr("Copy"));
  ctx_menu_.addMenu(submenu);
  ....
}

Предупреждение: V519 The 'submenu' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 287, 363. packet_list.cpp 363

В конструкторе динамически создаются элементы визуального интерфейса и добавляются в объектную иерархию Qt. Это позволяет проводить рекурсивное уничтожение созданных объектов при удалении объекта верхнего уровня. Однако один из пунктов меню так и не был добавлен в объектную иерархию, что приведет к утечке памяти.

Код:
void dissect_display_switch(gint offset, guint msg_len, ....)
{
  ....
  if((address_byte&DISPLAY_WRITE_ADDRESS_LINE_FLAG)
    !=DISPLAY_WRITE_ADDRESS_LINE_FLAG)
    offset+=1;msg_len-=1;
  ....
}

Предупреждение: V640 The code's operational logic does not correspond with its formatting. The second statement will always be executed. It is possible that curly brackets are missing. packet-unistim.c 1134

Не правильная расстановка фигурных скобок '{}' для выделения блоков условных операторов 'if' может приводить к ошибкам.

Тело условного оператора 'if' будет состоять из одной инструкции, хотя форматирование и логика программы требуют несколько. Чтобы исправить ошибку, необходимо заключить несколько инструкций в фигурные скобки '{}'.

Код:
void dissect_ssc_readposition (....)
{
  ....
  switch (service_action) {
  ....
  case LONG_FORM:
    if (!(flags & MPU)) {
    ....
    } else
      /*offset += 16;*/
      break;
    ....
  }
  ....
}

Предупреждение: V705 It is possible that 'else' block was forgotten or commented out, thus altering the program's operation logics. packet-scsi-ssc.c 831

Забавно, но всего лишь один комментарий, может привести к изменению логики работы программы. Выход из блока 'case LONG_FORM' будет выполнен только при срабатывании 'else', что неизбежно приведет к ошибке.

Код:
void set_has_console(gboolean set_has_console)
{
  has_console = has_console;
}

Предупреждение: V570 The 'has_console' variable is assigned to itself. console_win32.c 235

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

Код:
void dissect_dcc(tvbuff_t *tvb, packet_info *pinfo, 
                 proto_tree *tree, void *data _U_)
{
  client_is_le = ( (tvb_get_guint8(tvb, offset+4) 
    | tvb_get_guint8(tvb, offset+4)) 
    &&(tvb_get_guint8(tvb, offset+8) 
    | tvb_get_guint8(tvb, offset+9)) 
    && (tvb_get_guint8(tvb, offset+12) 
    | tvb_get_guint8(tvb, offset+13)) );
}

Предупреждение: V501 There are identical sub-expressions 'tvb_get_guint8(tvb, offset + 4)' to the left and to the right of the '|' operator. packet-dcc.c 272

Повторяется выражение tvb_get_guint8(tvb, offset+4). По аналогии можно предположить, что планировали написать tvb_get_guint8(tvb, offset+5).

Были и другие ошибки, о которых я не стал писать, чтоб не загромождать статью. Приведенных примеров должно быть достаточно, чтоб показать возможности статического анализа, и привлечь внимание людей к PVS-Studio. Если необходимо получить полное представление о возможностях PVS-Studio, на сайте можно найти список всех возможных предупреждений. Более тщательный анализ Wireshark могут осуществить сами разработчики. Им будет намного легче понять, является что-то ошибкой или нет.

Заключение


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

Желаю всем успехов в программировании и поменьше ошибок.


Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Kalashnikov. Static Analysis of Wireshark by PVS-Studio.

Прочитали статью и есть вопрос?
Часто к нашим статьям задают одни и те же вопросы. Ответы на них мы собрали здесь: Ответы на вопросы читателей статей про PVS-Studio и CppCat, версия 2015. Пожалуйста, ознакомьтесь со списком.

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


  1. AndreyKalashnikovViva64 Автор
    04.06.2015 19:58
    +3

    Мне будет очень приятно, если Вы поблагодарите меня и поставите Лайк. ))))

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

    Спасибо, за обратную связь! ))))))))


    1. AndreyKalashnikovViva64 Автор
      04.06.2015 20:03
      -1

      Тем более, что в этой статье удалось затронуть сразу три интересных темы: сетевые технологии, сборку open-source проектов и программирования на С++.


    1. Imp5
      04.06.2015 21:56
      +21

      Вместе с тем, я готов выслушать конструктивную критику

      У вас в тексте, в первой и последней строке закрывающие скобки, для которых отсутствуют открывающие.


      1. AndreyKalashnikovViva64 Автор
        04.06.2015 22:05
        -11

        Спасибо! Но я так выражаю эмоции. )))))))))


  1. Andrey2008
    04.06.2015 21:28
    +16

    А Вы отписали разработчикам о найденных дефектах?


    1. AndreyKalashnikovViva64 Автор
      04.06.2015 22:06
      +4

      Да. Разработчики уже знают об этой публикации.


      1. AlexP11223
        04.06.2015 23:00
        +4

        А под Линукс PVS будет?!111


        1. Andrey2008
          05.06.2015 00:22
          +2

          Отчасти он есть. Напишите нам с корпоративного ящика и мы обсудим этот вопрос подробнее.


          1. unwrecker
            05.06.2015 00:46
            +1

            А вы проверяли PVS-Studio с помощью PVS-Studio?


            1. ad1Dima
              05.06.2015 14:58

              Если это не шутка, то

              ЗЫ. я никак не связан с разработчиками PVS-Studio


    1. MaximChistov
      05.06.2015 00:15
      -1

      лол)))


  1. amarao
    04.06.2015 22:59

    Ещё tshark прогоните. Он у меня после 3-4 часов capture падает с segmentation fault, так что баг там точно есть.


    1. dmitrmax
      04.06.2015 23:38

      tshark — это всего лишь часть проекта wireshark.


      1. amarao
        05.06.2015 00:03

        Да, но wireshark не падает, а tshark падает. А проверяли ли код tshark'а — не понятно. Если проверяли, почему не поймали?


        1. AndreyKalashnikovViva64 Автор
          05.06.2015 00:08

          Какую версию и на какой операционной системе Вы используете?


          1. AndreyKalashnikovViva64 Автор
            05.06.2015 00:26

            В анализируемой версии был ряд ошибок, которые могли приводить к segmentation fault. Об этом мы написали разработчикам Wireshark. Но исправление ошибок — это их задача. Кроме того Вы сами можете им написать об этом: bugs.wireshark.org/bugzilla, ask.wireshark.org.


          1. amarao
            05.06.2015 00:44

            x86_64, debian sid. 1.12.5+g5819e5b-1


            1. JIghtuse
              05.06.2015 06:06

              С какими флагами запускали? Попытаюсь воспроизвести в gdb, у меня как раз та же версия установлена.


              1. amarao
                06.06.2015 08:55

                tshark -i eth0 -O http -Y http.request -T fields -e http.request.full_uri


                1. JIghtuse
                  06.06.2015 10:52

                  Вчера пробовал запускать без флагов — работало, пока не исчерпало место в корневом разделе. Сегодня попробую с флагами погонять.


        1. dmitrmax
          05.06.2015 11:00

          Мне кажется, что проверяли код проекта.

          wireshark отличается от tshark'а только интерфейсом, библиотека захвата и разбора пакетов у них общая.

          Кроме того, далеко не все баги можно найти статическим анализатором. Например, логический баг так не поймать.


  1. Andrey2008
    05.06.2015 09:15
    +1

    Пользуясь случаем, прошу всех, кому небезразличен PVS-Studio проголосовать вот за этот баг: Package Load Keys (PLK) generation page is broken two weeks месяц. Уже месяц не работает механизм подписи плагинов для Visual Studio, что будет нам мешать выпускать новые версии PVS-Studio. Спасибо всем, кто проголосует.


  1. AndreyKalashnikovViva64 Автор
    05.06.2015 14:53
    +1

    Разработчики знают о нашем анализе:
    Static Analysis of Wireshark by PVS-Studio