В этой статье я расскажу, как использовать PVS-Studio для статического анализа программного кода на языках С/C++ на примере open-source проекта Wireshark. Начну я с краткого описания анализатора сетевого трафика Wireshark и продукта PVS-Studio. Опишу подводные камни процесса сборки и подготовки проекта к статическому анализу. Постараюсь сформировать общую картину о продукте PVS-Studio, его преимуществах и удобстве использования, приводя предупреждения анализатора, примеры кода и собственные комментарии.
Для демонстрации возможностей PVS-Studio, требовалось найти известный, полезный и интересный open-source проект, анализ которого еще никто не делал. Я остановился на Wireshark, т.к. сам к нему не равнодушен, а если вы еще о нем не знаете, то возможно после прочтения этой статьи разделите мои чувства к нему.
Стремительное развитие интернета и большое количество фильмов о программистах-хакерах давно привлекло мое внимание к компьютерным сетям. А сейчас, мне кажется, что любому квалифицированному системному администратору и программисту, заботящемуся о безопасности, требуется разбираться в сетевых технологиях.
В основе сетей лежат прием и передача данных по определенным протоколам. Для того чтобы проводить исследования сетевых приложений и протоколов, а также, чтобы находить проблемы в работе сети, и, что важно, выяснять причины этих проблем, необходимо использовать инструменты для захвата и анализа сетевого трафика или снифферы.
Wireshark - это достаточно известный инструмент для захвата и анализа сетевого трафика с графическим интерфейсом пользователя. Программа основана на библиотеке для захвата сетевого трафика Pсap, и позволяет разбирать пакеты подавляющего большинства известных сетевых протоколов, отображая значение каждого поля протокола любого уровня.
Wireshark - кроссплатформенный инструмент, распространяемый под свободной лицензией GNU GPL, предназначенный для работы в Windows и Linux. Для формирования графического интерфейса используются библиотеки GTK+ и Qt.
Документацию и исходники программы можно найти на сайте.
Статический анализ кода позволяет находить ошибки в программном обеспечении без запуска приложений и вне зависимости от рабочего окружения. Используя статический анализ можно повысить качество программного продукта, сократить время на разработку и тестирование, а также обеспечить безопасность.
PVS-Studio - это статический анализатор C/C++/C++11 кода, поддерживающий компиляторы MS Visual С++, GNU GCC (MinGW), Clang, Borland C++.
PVS-Studio содержит следующие диагностики:
Дополнительную информацию о PVS-Studio можно найти на сайте.
Для проведения статического анализа скачаем исходники самой последней стабильной версии 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.
Дополнительную информацию о сборке можно найти на сайте.
Несмотря на то, что сборка проводилась в полном соответствии с инструкцией, возник ряд ошибок. Для их устранения потребовалось:
У меня установлена 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.
Для этого выполним ряд действий:
Ну вот мы и подошли к самому интересному - поиску ошибок.
Приступим к поиску ошибок, просматривая предупреждения 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, комментарии о котором мы видели. Поэтому всем рекомендую регулярно использовать статические анализаторы в своих проектах, чтоб выявлять ошибки ещё до тестирования на этапе написания кода.
Желаю всем успехов в программировании и поменьше ошибок.