Андрей Калашников

04 Июн 2015

Теги:

#Cpp

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

04 Июн 2015

Автор: Андрей Калашников

Анализатор сетевого трафика Wireshark
Статический анализатор кода PVS-Studio
Сборка проекта Wireshark
Проведение статического анализа средствами PVS-Studio
Поиск ошибок в проекте Wireshark
Заключение

В этой статье я расскажу, как использовать 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.

Для этого выполним ряд действий:

Создадим пустой проект Visual С++ в папке исходников Wireshark.
В Solution Explorer перейдем в режим отображения файлов.
Добавим исходники в наш проект.
Откроем 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, комментарии о котором мы видели. Поэтому всем рекомендую регулярно использовать статические анализаторы в своих проектах, чтоб выявлять ошибки ещё до тестирования на этапе написания кода.

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

#Cpp