Wireshark 3.x: تحليل الكود تحت نظام MacOS ومراجعة الأخطاء

أصدرت مؤسسة Wireshark النسخة الثابتة النهائية لمحلل حركة مرور الشبكة الشهير - Wireshark 3.0.0. يعمل الإصدار الجديد على إصلاح العديد من الأخطاء ، ومن الممكن الآن تحليل البروتوكولات الجديدة ، بصرف النظر عن استبدال برنامج التشغيل على Npcap WinPcap. هنا هو حيث ينتهي الاقتباس من الإعلان وتبدأ ملاحظتنا حول الأخطاء في المشروع. بالتأكيد لم يبذل مؤلفو المشروعات قصارى جهدهم في إصلاح الخلل قبل الإصدار.

دعونا نجمع الإصلاحات العاجلة الآن لإعطاء دافع في إصدار جديد :).

مقدمة

Wireshark هو أداة معروفة لالتقاط وتحليل حركة مرور الشبكة. يعمل البرنامج مع الغالبية العظمى من البروتوكولات المعروفة ، وله واجهة رسومية بديهية ومنطقية ، ونظام قوي للغاية من المرشحات. Wireshark هو نظام أساسي مشترك ، ويعمل في أنظمة تشغيل مثل: Windows و Linux و macOS و Solaris و FreeBSD و NetBSD وغيرها الكثير.

للقيام بتحليل الكود المصدري ، استخدمنا محلل الكود الثابت PVS-Studio . لتحليل الكود المصدري ، نحتاج أولاً إلى ترجمة المشروع في نظام التشغيل. كان الاختيار واسعًا ، ليس فقط بسبب طبيعة النظام الأساسي للمشروع ، ولكن أيضًا بسبب اختيار المحلل. اخترت ماك للتحليل. يمكنك أيضًا تشغيل المحلل تحت 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' غير صحيح دائمًا. packet-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 خذ بعين الاعتبار فحص '(اكتب == 0x1) || (اكتب! = 0x4) التعبير. التعبير مفرط أو يحتوي على خطأ مطبعي. packet-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; } 

تعتمد نتيجة العامل الشرطي فقط على هذا الجزء من التعبير (المصدر [إزاحة] == '') . الفحص (المصدر [إزاحة]! = '\ 0') زائدة ويمكن إزالته بأمان. ليس هذا هو الخطأ الفعلي ، لكن الكود الزائد يجعل قراءة الكود وفهم البرنامج أكثر صعوبة ، لذلك من الأفضل تبسيطه.

تحذير 4

التعبير V547 'eras_pos! = NULL' صحيح دائمًا. 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]); } } .... } 

ربما ، نحن نتعامل مع فحص زائد ، ربما مع خطأ مطبعي ، ويجب فحص شيء آخر في أحد شروط كتلة if .

تأكيدات غريبة

تحذير 1

تعبير V547 'sub_dissectors! = NULL' صحيح دائمًا. 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 "أنا <عدد" صحيح دائمًا. packet-netflow.c 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 تم استخدام مؤشر 'si-> conv' قبل أن يتم التحقق منه ضد nullptr. خطوط الفحص: 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 تم استخدام مؤشر 'protos' بعد إصدار الذاكرة. packet-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 في مرحلة ما. لم يتم ذلك في مقتطف الشفرة المحدد ويتم تخصيص جزء جديد من الذاكرة في الحلقة على كل تكرار. هنا يأتي تسرب الذاكرة متعددة.

بعض التحذيرات الأخرى لشظايا الكود المشابهة:

• 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 دائمًا في نصوص الوظيفة قبل استخدامها. packet-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 من الممكن أن يتم إلغاء وظيفة افتراضية بشكل غير صحيح. راجع الوسيطة الثالثة للدالة "headerData" في الفئة المشتقة "PacketListModel" والفئة الأساسية "QAbstractItemModel". 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 . الدالات لها قيم افتراضية مختلفة لمعلمة الدور . هذا يمكن أن يسبب السلوك الخاطئ ، وليس السلوك المتوقع من قبل مبرمج.

تحذير 4

V610 سلوك غير محدد. تحقق من مشغل التحول '>>'. المعامل الأيمن ('bitshift' = [0..64]) أكبر من أو يساوي الطول في أجزاء المعامل الأيسر المعزز. proto.c 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.