The Wireshark Foundation telah merilis versi stabil terakhir dari penganalisa lalu lintas jaringan populer - Wireshark 3.0.0. Rilis baru memperbaiki beberapa bug, mengimplementasikan kemampuan untuk menganalisis protokol baru dan mengganti driver WinPcap dengan Npcap. Di sini kutipan pengumuman berakhir dan artikel kami tentang bug dalam proyek dimulai. Sebelum rilis, mereka jelas tidak cukup diperbaiki. Mari kita perbaiki sehingga ada alasan untuk membuat rilis baru :).
Pendahuluan
Wireshark adalah alat yang terkenal untuk menangkap dan menganalisis lalu lintas jaringan. Program ini bekerja dengan sebagian besar protokol yang diketahui, memiliki antarmuka grafis yang jelas dan logis, sistem filter yang kuat. Wireshark - lintas-platform, bekerja di OS seperti: Windows, Linux, macOS, Solaris, FreeBSD, NetBSD dan banyak lainnya.
Untuk mencari kesalahan, kami menggunakan penganalisa statis
PVS-Studio . Untuk menganalisis kode sumber, Anda harus terlebih dahulu mengkompilasi proyek di beberapa sistem operasi. Pilihannya bagus bukan hanya karena sifat lintas-platform dari proyek, tetapi juga karena sifat lintas-platform dari penganalisa. Untuk menganalisis proyek, saya memilih macOS. Alat analisis juga dapat diluncurkan pada Windows dan Linux.
Tentang kualitas kode yang ingin saya sampaikan secara terpisah. Sayangnya, saya tidak bisa menyebutnya bagus. Ini adalah penilaian subjektif, tetapi karena kami secara teratur
memeriksa banyak proyek, saya memiliki sesuatu untuk dibandingkan. Dalam hal ini, sejumlah besar peringatan PVS-Studio pada sejumlah kecil kode sangat mencolok. Secara total, lebih dari 3.500 peringatan dari semua tingkatan telah dikeluarkan untuk proyek ini. Ini tipikal untuk proyek yang tidak menggunakan alat analisis statis sama sekali, bahkan yang gratis. Faktor lain yang menunjukkan kualitas proyek adalah kesalahan berulang yang diidentifikasi oleh penganalisa. Contoh kode yang sama tidak akan diberikan dalam artikel, tetapi beberapa kesalahan identik ada dalam kode di ratusan tempat.
Sisipan semacam itu tidak menambah kualitas pada kode:
#line 1 "./asn1/acse/packet-acse-template.c"
Ada lebih dari 1000 di seluruh proyek. Sisipan semacam itu mempersulit penganalisa untuk membandingkan peringatan yang dihasilkan dengan file yang diinginkan. Tapi saya yakin pengembang biasa tidak menikmati dukungan kode seperti itu.
Salah ketik
Peringatan 1V641 Ukuran buffer memori yang dialokasikan bukan kelipatan dari ukuran elemen. 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)); .... }
Ada dua jenis struktur:
mate_cfg_gog dan
mate_cfg_gop , mereka sangat mirip, tetapi tidak identik. Kemungkinan besar, fungsi-fungsi tercampur dalam fragmen kode ini, yang penuh dengan potensi kesalahan dalam program selama akses memori oleh pointer.
Berikut ini adalah potongan-potongan struktur data yang bingung:
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;
Peringatan 2V519 Variabel 'HDR_TCP.dest_port' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 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); .... }
Di baris terakhir, nilai yang dihitung hanya dari variabel
HDR_TCP.dest_port tidak
dikonfirmasi .
Kesalahan logis
Pada bagian ini saya akan memberikan beberapa contoh kesalahan dalam pernyataan bersyarat, dan semuanya akan berbeda secara mendasar satu sama lain.
Peringatan 1Ekspresi V547 'direction == 0' selalu salah. paket-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;
Dalam kondisi eksternal, variabel
arah dibandingkan dengan
P2P_DIR_RECV konstan. Menulis ekspresi melalui operator AND berarti bahwa ketika kondisi internal tercapai, nilai variabel
arah pasti tidak akan sama dengan
P2P_DIR_SENT konstan
lainnya .
Peringatan 2V590 Pertimbangkan untuk memeriksa '(type == 0x1) || (ketik! = 0x4) ekspresi. Ekspresi berlebihan atau mengandung kesalahan cetak. package-fcsb3.c 686
static int dissect_fc_sbccs (....) { .... else if ((type == FC_SBCCS_IU_CMD_HDR) || (type != FC_SBCCS_IU_CMD_DATA)) { .... }
Kesalahan fragmen kode ini adalah bahwa hasil dari kondisi tergantung hanya pada satu ekspresi:
(type != FC_SBCCS_IU_CMD_DATA)
Peringatan 3V590 Pertimbangkan untuk memeriksa ungkapan ini. Ekspresi berlebihan atau mengandung kesalahan cetak. snort-config.c 40
static char *skipWhiteSpace(char *source, int *accumulated_offset) { int offset = 0; while (source[offset] != '\0' && source[offset] == ' ') { offset++; } *accumulated_offset += offset; return source + offset; }
Hasil pernyataan kondisional hanya akan bergantung pada bagian ekspresi ini
(sumber [offset] == '') . Pemeriksaan
(sumber [offset]! = '\ 0') berlebihan dan dapat dihapus dengan aman. Ini bukan kesalahan nyata, tetapi kode yang berlebihan membuatnya sulit untuk membaca dan memahami program, jadi lebih baik untuk menyederhanakannya.
Peringatan 4Ekspresi V547 'eras_pos! = NULL' selalu benar. 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]); } } .... }
Mungkin kita berhadapan dengan verifikasi yang tidak perlu, dan mungkin dengan kesalahan ketik, dan dalam salah satu jika, ada hal lain yang harus diperiksa.
Aset Aneh
Peringatan 1V547 Ekspresi 'sub_dissectors! = NULL' selalu benar. 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);
Memeriksa pointer di
g_assert pada saat ini berlebihan, karena pointer diperiksa sebelum itu. Mungkin dalam fungsi ini hanya ada
g_assert sebelumnya , dan mereka lupa menghapusnya, tapi mungkin di sini Anda harus memeriksa beberapa bidang struktur.
Peringatan 2Ekspresi V547 'i <count' selalu benar. 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);
Tidak jelas mengapa fungsi memiliki pernyataan yang menduplikasi kondisi dari satu loop. Penghitung siklus di tubuh tidak berubah.
Kesalahan dengan pointer
Peringatan 1V595 Pointer 'si-> conv' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 2135, 2144. package-smb2.c 2135
static int dissect_smb2_fid(....) { .... g_hash_table_insert(si->conv->fids, sfi, sfi);
Pointer
si-> conv didereferensi beberapa garis lebih awal daripada yang diperiksa apakah sama dengan nol atau tidak.
Peringatan 2V774 Penunjuk 'protos' digunakan setelah memori dilepaskan. paket-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 adalah array string. Selama memproses pengecualian dalam program, array ini pertama kali dihapus oleh fungsi
g_strfreev , dan kemudian salah satu baris array ini digunakan dalam pesan kesalahan. Kemungkinan besar, baris-baris ini dalam kode harus dipertukarkan:
*err = g_strdup_printf("Could not find dissector for: '%s'", protos[i]); g_strfreev(protos);
Memori bocor
V773 Pointer 'ptmpstr' diberi nilai dua kali tanpa melepaskan memori. Kebocoran memori dimungkinkan. 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); } .... }
Setelah fungsi
g_strdup ,
Anda perlu memanggil fungsi
g_free di beberapa titik. Dalam fragmen kode yang disajikan, ini tidak dilakukan, dan dalam loop di setiap iterasi, sepotong RAM baru dialokasikan. Banyak kebocoran memori terjadi.
Beberapa peringatan lagi untuk cuplikan kode serupa:
- V773 Pointer 'ptmpstr' diberi nilai dua kali tanpa melepaskan memori. Kebocoran memori dimungkinkan. idl2wrs.c 2447
- V773 Pointer 'ptmpstr' diberi nilai dua kali tanpa melepaskan memori. Kebocoran memori dimungkinkan. idl2wrs.c 2713
- V773 Pointer 'ptmpstr' diberi nilai dua kali tanpa melepaskan memori. Kebocoran memori dimungkinkan. idl2wrs.c 2728
- V773 Pointer 'ptmpstr' diberi nilai dua kali tanpa melepaskan memori. Kebocoran memori dimungkinkan. idl2wrs.c 2732
- V773 Pointer 'ptmpstr' diberi nilai dua kali tanpa melepaskan memori. Kebocoran memori dimungkinkan. idl2wrs.c 2745
Sayangnya, ada banyak tempat lain yang serupa dalam kode di mana memori tidak dibebaskan.
Lain-lain
Peringatan 1V535 Variabel 'i' digunakan untuk loop ini dan untuk loop luar. Periksa baris: 7716, 7798. package-opa-mad.c 7798
static gint parse_GetVFInfo(....) { .... for (i = 0; i < records; i++) {
Dalam fungsi yang sangat panjang, pengembang dengan berani mengubah nilai penghitung lingkaran, dan melakukan ini beberapa kali. Sulit untuk mengatakan apakah ini kesalahan atau tidak, tetapi ada sekitar 10 siklus dalam proyek tersebut.
Peringatan 2V763 Parameter 'item' selalu ditulis ulang di badan fungsi sebelum digunakan. package-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, ....);
Pointer
item yang diambil fungsi segera rusak oleh nilai lain. Ini sangat mencurigakan. Selain itu, kode tersebut berisi beberapa lusin tempat seperti itu, sehingga sulit untuk mengatakan apakah ini kesalahan atau tidak. Saya bertemu dengan kode yang sama sebelumnya di proyek besar lainnya, itu kode yang benar, hanya saja tidak ada yang berani mengubah antarmuka fungsi.
Peringatan 3V762 Mungkin saja fungsi virtual diganti secara tidak benar. Lihat argumen ketiga fungsi 'headerData' di kelas turunan 'PacketListModel' dan kelas dasar 'QAbstractItemModel'. package_list_model.h 48
QVariant QAbstractItemModel::headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const
Analyzer mendeteksi kelebihan fungsi
headerData yang salah . Fungsi memiliki nilai default yang berbeda untuk parameter
peran . Ini mungkin tidak mengarah pada perilaku yang diharapkan oleh programmer.
Peringatan 4V610 Perilaku tidak terdefinisi. Periksa operator shift '>>'. Operan kanan ('bitshift' = [0..64]) lebih besar dari atau sama dengan panjang dalam bit dari operan kiri yang dipromosikan. 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; .... }
Pergeseran 64-bit akan menghasilkan perilaku yang tidak terdefinisi sesuai dengan standar bahasa.
Sebaliknya, kode yang benar harus seperti ini:
if (bitshift == 64) available_bits = 0; else available_bits = G_GUINT64_CONSTANT(0xFFFFFFFFFFFFFFFF) >> bitshift;
Kesimpulan
Kelihatannya ada beberapa contoh kesalahan dalam tinjauan, tetapi dalam laporan lengkap kasus yang disajikan berulang puluhan dan ratusan kali. Ikhtisar peringatan PVS-Studio hanya untuk tujuan demonstrasi. Ini merupakan kontribusi terhadap kualitas proyek sumber terbuka, tetapi pemeriksaan satu kali adalah cara yang paling tidak efisien untuk menerapkan metodologi analisis statis.
Anda bisa mendapatkan dan menganalisis sendiri laporan lengkapnya. Untuk melakukan ini, Anda hanya perlu
mengunduh dan menjalankan alat analisa PVS-Studio.
Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Svyatoslav Razmyslov.
Wireshark 3.x: analisis kode di bawah tinjauan macOS dan kesalahan