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

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

مقدمة

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]); } } .... } 

ربما نتعامل مع التحقق غير الضروري ، وربما مع خطأ مطبعي ، وفي إحدى الحالات ، يجب فحص شيء آخر.

أصول غريبة

تحذير 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-> عدة أسطر في وقت أبكر مما يتم التحقق مما إذا كان يساوي الصفر أم لا.

تحذير 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

لسوء الحظ ، هناك العديد من الأماكن الأخرى المشابهة في الكود حيث لا يتم تحرير الذاكرة.

miscellanea

تحذير 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.

إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فالرجاء استخدام الرابط الخاص بالترجمة: Svyatoslav Razmyslov. Wireshark 3.x: تحليل الكود تحت نظام MacOS ومراجعة الأخطاء