Geekly articles weekly

Wireshark 3.x：macOS代码分析和错误检查

图片1

Wireshark Foundation已发布了流行的网络流量分析器的最终稳定版本-Wireshark 3.0.0。 新版本修复了多个错误，实现了分析新协议的功能，并用Npcap替换了WinPcap驱动程序。 在这里，声明的引用结束，并且我们在项目中的bug的文章开始了。 在发布之前，他们显然没有得到足够的纠正。 让我们进行一些修复，以便有理由发布一个新版本:)。

引言


Wireshark是用于捕获和分析网络流量的著名工具。 该程序与绝大多数已知协议兼容，具有清晰逻辑的图形界面和强大的过滤系统。 Wireshark-跨平台，可在以下操作系统中使用：Windows，Linux，macOS，Solaris，FreeBSD，NetBSD等。

为了搜索错误，我们使用了PVS-Studio静态分析仪。 要分析源代码，必须首先在某些操作系统中编译项目。 之所以选择这个项目，不仅因为该项目具有跨平台性质，而且还因为该分析仪具有跨平台性质。 为了对该项目进行分析，我选择了macOS。 该分析仪也可以在Windows和Linux上启动。

我想分别谈谈代码的质量。 不幸的是，我不能称其为好。 这是一个主观评估，但是由于我们会定期检查很多项目，因此我有一些值得比较的地方。 在这种情况下，使用少量代码的大量PVS-Studio警告会引起轰动。 该项目总共发出了3500多个各级警告。 对于完全不使用静态分析工具甚至免费工具的项目来说，这是典型的。 指示项目质量的另一个因素是分析仪发现的重复错误。 本文不会提供相同的代码示例，但是在数百个位置的代码中都存在一些相同的错误。

同样，这样的插入不会增加代码的质量：

/* Input file: packet-acse-template.c */ #line 1 "./asn1/acse/packet-acse-template.c"

整个项目中有1000多个。 这样的插入使分析仪很难将生成的警告与所需文件进行比较。 但是我敢肯定，普通开发人员不会享受这种代码的支持。

错别字


警告1

V641分配的内存缓冲区的大小不是元素大小的倍数。 mate_setup.c 100

 extern mate_cfg_gog* new_gogcfg(mate_config* mc, gchar* name) { mate_cfg_gog* cfg = (mate_cfg_gog *)g_malloc(sizeof(mate_cfg_gop)); .... }

有两种类型的结构： mate_cfg_gog和mate_cfg_gop ，它们非常相似，但不完全相同。 最有可能的是，这些函数混在了这段代码片段中，在通过指针进行内存访问期间，程序中充满了潜在的错误。

以下是混淆的数据结构的片段：

 typedef struct _mate_cfg_gog { gchar* name; GHashTable* items; guint last_id; GPtrArray* transforms; LoAL* keys; AVPL* extra; float expiration; gop_tree_mode_t gop_tree_mode; gboolean show_times; .... } mate_cfg_gog; typedef struct _mate_cfg_gop { gchar* name; guint last_id; GHashTable* items; GPtrArray* transforms; gchar* on_pdu; AVPL* key; AVPL* start; AVPL* stop; AVPL* extra; float expiration; float idle_timeout; float lifetime; gboolean drop_unassigned; gop_pdu_tree_t pdu_tree_mode; gboolean show_times; .... } mate_cfg_gop;

警告2

V519为“ HDR_TCP.dest_port”变量连续两次分配值。 也许这是一个错误。 检查行：495，496。text_import.c 496

 void write_current_packet (void) { .... HDR_TCP.source_port =isOutbound ? g_htons(hdr_dest_port):g_htons(hdr_src_port); HDR_TCP.dest_port = isOutbound ? g_htons(hdr_src_port) :g_htons(hdr_dest_port); HDR_TCP.dest_port = g_htons(hdr_dest_port); .... }

在最后一行中， 未确认HDR_TCP.dest_port变量的刚刚计算出的值。

逻辑错误


在本节中，我将提供一些条件语句中的错误的示例，所有这些错误在根本上都是彼此不同的。

警告1

V547表达式“ direction == 0”始终为false。 包-adb.c 291

 #define P2P_DIR_RECV 1 #define P2P_DIR_SENT 0 static void save_command(....) { .... if ( service_data && service_data->remote_id == 0 && direction == P2P_DIR_RECV) { if (direction == P2P_DIR_SENT) { service_data->remote_id = arg1; // unreachable code } else { service_data->remote_id = arg0; } .... } .... }

在外部条件下，将方向变量与常量P2P_DIR_RECV进行比较。 通过AND运算符编写表达式意味着，当达到内部条件时， 方向变量的值将不完全等于另一个常量P2P_DIR_SENT 。

警告2

V590考虑检查'（type == 0x1）|| （type！= 0x4）'表达式。 表达式过多或打印错误。 包-fcsb3.c 686

 static int dissect_fc_sbccs (....) { .... else if ((type == FC_SBCCS_IU_CMD_HDR) || (type != FC_SBCCS_IU_CMD_DATA)) { .... }

此代码段的错误是条件的结果仅取决于一个表达式：

 (type != FC_SBCCS_IU_CMD_DATA)

警告3

V590考虑检查此表达式。 表达式过多或打印错误。 snort-config.c 40

 static char *skipWhiteSpace(char *source, int *accumulated_offset) { int offset = 0; /* Skip any leading whitespace */ while (source[offset] != '\0' && source[offset] == ' ') { offset++; } *accumulated_offset += offset; return source + offset; }

条件语句的结果将仅取决于表达式的这一部分（source [offset] ==''） 。 校验（源[offset]！='\ 0'）是多余的，可以安全地删除。 这不是一个真正的错误，但是冗余代码使阅读和理解程序变得困难，因此最好对其进行简化。

警告4

V547表达式“ eras_pos！= NULL”始终为true。 reedsolomon.c 659

 int eras_dec_rs(dtype data[NN], int eras_pos[NN-KK], int no_eras) { .... if(eras_pos != NULL){ for(i=0;i<count;i++){ if(eras_pos!= NULL) eras_pos[i] = INDEX_TO_POS(loc[i]); } } .... }

也许我们正在处理不必要的验证，可能还有错字，并且在其中一种情况下，应该检查其他内容。

奇怪的资产


警告1

V547表达式“ sub_dissectors！= NULL”始终为true。 capture_dissectors.c 129

 void capture_dissector_add_uint(....) { .... sub_dissectors = (struct capture_dissector_table*)g_hash_table_lookup(....); if (sub_dissectors == NULL) { fprintf(stderr, "OOPS: Subdissector \"%s\" not found ... \n", name); if (getenv("WIRESHARK_ABORT_ON_DISSECTOR_BUG") != NULL) abort(); return; } g_assert(sub_dissectors != NULL); // <= .... }

此时，在g_assert中检查指针是多余的，因为在此之前已检查了指针。 也许在此函数中，以前只有g_assert ，他们忘记了删除它，但是也许在这里您应该检查一些结构字段。

警告2

V547表达式'i <count'始终为true。 数据包网络流10363

 static int dissect_v9_v10_template_fields(....) { .... count = tmplt_p->field_count[fields_type]; for(i=0; i<count; i++) { .... if (tmplt_p->fields_p[fields_type] != NULL) { DISSECTOR_ASSERT (i < count); // <= tmplt_p->fields_p[fields_type][i].type = type; tmplt_p->fields_p[fields_type][i].length = length; tmplt_p->fields_p[fields_type][i].pen = pen; tmplt_p->fields_p[fields_type][i].pen_str = pen_str; if (length != VARIABLE_LENGTH) {/ tmplt_p->length += length; } } .... } .... }

尚不清楚为什么在函数中存在断言 ，该断言从循环中复制了条件。 体内的循环计数器不变。

指针错误


警告1

V595在针对nullptr进行验证之前，已使用“ si-> conv”指针。 检查行：2135，2144。packet-smb2.c 2135

 static int dissect_smb2_fid(....) { .... g_hash_table_insert(si->conv->fids, sfi, sfi); // <= si->file = sfi; if (si->saved) { si->saved->file = sfi; si->saved->policy_hnd = policy_hnd; } if (si->conv) { // <= eo_file_info = (.... *)g_hash_table_lookup(si->conv->files,&policy_hnd); .... } .... }

在检查指针si-> conv是否等于零之前将其取消引用几行。

警告2

V774释放内存后使用了“原型”指针。 数据包-k12.c 311

 static gboolean k12_update_cb(void* r, char** err) { gchar** protos; .... for (i = 0; i < num_protos; i++) { if ( ! (h->handles[i] = find_dissector(protos[i])) ) { h->handles[i] = data_handle; h->handles[i+1] = NULL; g_strfreev(protos); *err = g_strdup_printf("Could not find dissector for: '%s'", protos[i]); return FALSE; } } .... }

protos是一个字符串数组。 在程序中处理异常期间，该数组首先由g_strfreev函数清除，然后在错误消息中使用该数组的一行。 代码中的这些行很可能应该互换：

 *err = g_strdup_printf("Could not find dissector for: '%s'", protos[i]); g_strfreev(protos);

内存泄漏


V773'ptmpstr'指针被分配了两次值，而没有释放内存。 可能发生内存泄漏。 idl2wrs.c 2436

 static void parsetypedefunion(int pass) { char tmpstr[BASE_BUFFER_SIZE], *ptmpstr; .... while(num_pointers--){ g_snprintf(tmpstr, BASE_BUFFER_SIZE, "%s_%s", ptmpstr, "unique"); FPRINTF(eth_code, "static int\n"); FPRINTF(eth_code, "....", tmpstr); FPRINTF(eth_code, "{\n"); FPRINTF(eth_code, " ....", ptmpstr, ti->str); FPRINTF(eth_code, " return offset;\n"); FPRINTF(eth_code, "}\n"); FPRINTF(eth_code, "\n"); ptmpstr=g_strdup(tmpstr); } .... }

使用g_strdup函数后， 您需要在某个时候调用g_free函数。 在给出的代码段中，这没有完成，并且在每次迭代的循环中分配了一块新的RAM。 发生多个内存泄漏。

类似代码段的一些警告：

  • V773'ptmpstr'指针被分配了两次值，而没有释放内存。 可能发生内存泄漏。 idl2wrs.c 2447
  • V773'ptmpstr'指针被分配了两次值，而没有释放内存。 可能发生内存泄漏。 idl2wrs.c 2713
  • V773'ptmpstr'指针被分配了两次值，而没有释放内存。 可能发生内存泄漏。 idl2wrs.c 2728
  • V773'ptmpstr'指针被分配了两次值，而没有释放内存。 可能发生内存泄漏。 idl2wrs.c 2732
  • V773'ptmpstr'指针被分配了两次值，而没有释放内存。 可能发生内存泄漏。 idl2wrs.c 2745

不幸的是，代码中还有许多其他类似的地方没有释放内存。

杂项


警告1

V535变量“ i”用于该循环和外部循环。 检查行：7716，7798。packet-opa-mad.c 7798

 /* Parse GetVFInfo MAD from the Performance Admin class. */ static gint parse_GetVFInfo(....) { .... for (i = 0; i < records; i++) { // <= line 7716 .... for (i = 0; i < PM_UTIL_BUCKETS; i++) { // <= line 7748 GetVFInfo_Util_Stats_Bucket_item = proto_tree_add_item(....); proto_item_set_text(....); local_offset += 4; } .... for (i = 0; i < PM_ERR_BUCKETS; i++) { // <= line 7798 GetVFInfo_Error_Stats_Bucket_item = proto_tree_add_item(....); proto_item_set_text(....); local_offset += 4; .... } .... } .... }

在很长的功能中，开发人员大胆地更改循环计数器的值，然后执行几次。 很难说这是否是一个错误，但是在项目中大约有10个这样的周期。

警告2

V763参数“ item”在使用前总是在功能体中重写。 包cdma2k.c 1324

 static void cdma2k_message_ORDER_IND(proto_item *item, ....) { guint16 addRecLen = -1, ordq = -1, rejectedtype = -1; guint16 l_offset = -1, rsc_mode_ind = -1, ordertype = -1; proto_tree *subtree = NULL, *subtree1 = NULL; item = proto_tree_add_item(tree,hf_cdma2k_OrderIndMsg, tvb, ....); // <= subtree = proto_item_add_subtree(item, ett_cdma2k_subtree1); .... }

该函数采用的项目指针会立即被另一个值磨损。 这是非常可疑的。 而且，代码中包含数十个这样的位置，因此很难说这是否是一个错误。 在另一个大型项目中，我早些时候遇到过类似的代码，那里是正确的代码，只是没人敢更改函数的接口。

警告3

V762虚函数可能被错误地覆盖。 请参见派生类“ PacketListModel”和基类“ QAbstractItemModel”中函数“ headerData”的第三个参数。 packet_list_model.h 48

 QVariant QAbstractItemModel::headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const // <= class PacketListModel : public QAbstractItemModel { Q_OBJECT public: .... QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole | Qt::ToolTipRole) const; // <= .... };

分析器检测到headerData函数的错误重载。 函数的role参数具有不同的默认值。 这可能不会导致程序员期望的行为。

警告4

V610未定义的行为。 检查移位运算符“ >>”。 右操作数（'bitshift'= [0..64]）大于或等于提升后的左操作数的位长度。 协议10941

 static gboolean proto_item_add_bitmask_tree(...., const int len, ....) { .... if (len < 0 || len > 8) g_assert_not_reached(); bitshift = (8 - (guint)len)*8; available_bits = G_GUINT64_CONSTANT(0xFFFFFFFFFFFFFFFF) >> bitshift; .... }

根据语言标准，移位64位将导致不确定的行为。

正确的代码应该是这样的：

 if (bitshift == 64) available_bits = 0; else available_bits = G_GUINT64_CONSTANT(0xFFFFFFFFFFFFFFFF) >> bitshift;

结论


审查中似乎没有错误的例子，但是在完整的报告中，所提出的案例被重复了数十百次。 PVS-Studio警告概述仅用于演示目的。 这是对开源项目质量的一项贡献，但是一次性检查是应用静态分析方法最无效的方法。

您可以自己获取和分析完整报告。 为此，您只需要下载并运行PVS-Studio分析仪。



如果您想与说英语的读者分享这篇文章，请使用以下链接：Svyatoslav Razmyslov。 Wireshark 3.x：macOS下的代码分析和错误检查

Source: https://habr.com/ru/post/zh-CN447158/

More articles:


All Articles