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



    В этой статье я расскажу, как использовать 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. Пожалуйста, ознакомьтесь со списком.
    PVS-Studio 165,21
    Ищем ошибки в C, C++ и C# на Windows и Linux
    Поделиться публикацией
    Похожие публикации
    Комментарии 24
    • Мне будет очень приятно, если Вы поблагодарите меня и поставите Лайк. ))))

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

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

          У вас в тексте, в первой и последней строке закрывающие скобки, для которых отсутствуют открывающие.
      • +16
        А Вы отписали разработчикам о найденных дефектах?
      • 0
        Ещё tshark прогоните. Он у меня после 3-4 часов capture падает с segmentation fault, так что баг там точно есть.
        • 0
          tshark — это всего лишь часть проекта wireshark.
          • 0
            Да, но wireshark не падает, а tshark падает. А проверяли ли код tshark'а — не понятно. Если проверяли, почему не поймали?
            • Какую версию и на какой операционной системе Вы используете?
              • В анализируемой версии был ряд ошибок, которые могли приводить к segmentation fault. Об этом мы написали разработчикам Wireshark. Но исправление ошибок — это их задача. Кроме того Вы сами можете им написать об этом: bugs.wireshark.org/bugzilla, ask.wireshark.org.
                • 0
                  x86_64, debian sid. 1.12.5+g5819e5b-1
                  • 0
                    С какими флагами запускали? Попытаюсь воспроизвести в gdb, у меня как раз та же версия установлена.
                    • 0
                      tshark -i eth0 -O http -Y http.request -T fields -e http.request.full_uri
                      • 0
                        Вчера пробовал запускать без флагов — работало, пока не исчерпало место в корневом разделе. Сегодня попробую с флагами погонять.
                • 0
                  Мне кажется, что проверяли код проекта.

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

                  Кроме того, далеко не все баги можно найти статическим анализатором. Например, логический баг так не поймать.
            • +1
              Пользуясь случаем, прошу всех, кому небезразличен PVS-Studio проголосовать вот за этот баг: Package Load Keys (PLK) generation page is broken two weeks месяц. Уже месяц не работает механизм подписи плагинов для Visual Studio, что будет нам мешать выпускать новые версии PVS-Studio. Спасибо всем, кто проголосует.
              • Разработчики знают о нашем анализе:
                Static Analysis of Wireshark by PVS-Studio

                Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

                Самое читаемое