Wireshark Foundation merilis versi stabil terakhir dari penganalisa lalu lintas jaringan populer - Wireshark 3.0.0. Rilis baru memperbaiki beberapa bug, sekarang mungkin untuk menganalisis protokol baru, selain itu driver pada Npcap WinPcap diganti. Di sinilah kutipan pengumuman berakhir dan catatan kami tentang bug dalam proyek dimulai. Para penulis proyek pasti belum melakukan yang terbaik dalam memperbaiki bug sebelum rilis.
Mari kumpulkan perbaikan terbaru sekarang untuk memberikan motif dalam melakukan rilis baru :).
Pendahuluan
Wireshark adalah alat yang terkenal untuk menangkap dan menganalisis lalu lintas jaringan. Program ini bekerja dengan sebagian besar protokol yang dikenal, memiliki antarmuka grafis yang intuitif dan logis, sistem filter yang sangat canggih. Wireshark adalah lintas-platform, berfungsi dalam OS semacam itu, seperti: Windows, Linux, macOS, Solaris, FreeBSD, NetBSD, dan banyak lainnya.
Untuk melakukan analisis kode sumber, kami menggunakan penganalisa kode statis
PVS-Studio . Untuk menganalisis kode sumber, pertama-tama kita perlu mengkompilasi proyek dalam OS. Pilihannya luas bukan hanya karena sifat lintas platform dari proyek, tetapi juga karena analisa. Saya memilih macOS untuk analisis. Anda juga dapat menjalankan analisa di Windows dan Linux.
Saya ingin menarik perhatian khusus pada kualitas kode. Sayangnya, saya tidak bisa memberikan poin besar untuk itu. Ini adalah penilaian subjektif, tetapi karena kami secara teratur memeriksa banyak proyek, saya memiliki kerangka acuan. Yang menonjol dalam kasus ini adalah sejumlah besar peringatan PVS-Studio untuk sejumlah kecil kode. Secara total, lebih dari 3500 peringatan dari semua tingkatan dipicu untuk proyek ini. Ini tipikal untuk proyek, yang umumnya tidak menggunakan alat analisis statis, bahkan yang gratis. Faktor lain yang menunjuk pada kualitas proyek adalah kesalahan berulang yang terdeteksi oleh penganalisa. Saya tidak akan mengutip contoh kode jenis yang sama, sedangkan beberapa kesalahan serupa terjadi di ratusan tempat.
Sisipan semacam itu juga tidak memberikan peningkatan kualitas kode:
#line 1 "./asn1/acse/packet-acse-template.c"
Ada lebih dari 1000 di seluruh proyek. Sisipan semacam itu mempersulit penganalisis untuk mencocokkan peringatan yang dikeluarkan dengan file yang sesuai. Yah, saya pikir pengembang rata-rata tidak akan mendapatkan keluar dari mempertahankan kode tersebut.
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 tipe struktur:
mate_cfg_gog dan
mate_cfg_gop, mereka sangat mirip, tetapi tidak sama. Kemungkinan besar, dalam fungsi fragmen kode ini digabungkan, yang penuh dengan potensi kesalahan dalam program saat mengakses memori oleh pointer.
Berikut adalah fragmen dari struktur data yang tercampur aduk:
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 baru saja dievaluasi) dari variabel
HDR_TCP.dest_port ditulis ulang.
Kesalahan logis
Di bagian ini, saya akan mengutip beberapa contoh kesalahan dalam operator bersyarat, dan semuanya akan sangat berbeda 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
. Menurut ekspresi yang ditulis dengan operator AND, ketika sampai ke kondisi dalam, nilai
arah variabel pasti akan berbeda dari
P2P_DIR_SENT konstan lain.
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 hanya bergantung 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 dari operator kondisional hanya akan bergantung pada bagian ekspresi ini
(sumber [offset] == '') . Pemeriksaan
(sumber [offset]! = '\ 0') berlebihan dan dapat dihapus dengan aman. Ini bukan kesalahan sebenarnya, tetapi kode yang berlebihan membuat pembacaan kode dan memahami program lebih sulit, 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 berurusan dengan cek yang berlebihan, mungkin dengan kesalahan ketik, dan satu hal lagi harus diperiksa di salah satu kondisi blok
if .
Aneh menegaskan
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);
Pemeriksaan pointer
g_assert berlebihan di sini, karena pointer sudah diperiksa sebelumnya. Mungkin, hanya
g_assert yang menggunakan fungsi ini dan pengembang lupa untuk menghapusnya, tetapi mungkin bidang struktur seharusnya sudah diperiksa di sini.
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 cukup jelas mengapa pernyataan, yang menduplikasi kondisi dari loop, terjadi dalam fungsi. Penghitung lingkaran tidak akan berubah di tubuh.
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 mendapat dereferenced beberapa baris sebelum dicek null.
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. Ketika menangani kasus khusus dalam program ini, array ini pertama kali dihapus oleh fungsi
g_strfreev dan kemudian satu string array ini digunakan dalam pesan kesalahan. Kemungkinan besar, baris-baris ini 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, kita perlu memanggil fungsi
g_free di beberapa titik. Itu tidak dilakukan dalam potongan kode yang diberikan dan bagian memori baru dialokasikan dalam loop pada setiap iterasi. Inilah beberapa kebocoran memori.
Beberapa peringatan lain untuk fragmen kode yang 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, dalam kode ada banyak kasus serupa lainnya, di mana memori dilepaskan.
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 lama, pengembang dengan berani mengubah nilai penghitung loop, bahkan melakukannya beberapa kali. Kami tidak bisa mengatakan dengan pasti apakah itu kesalahan atau tidak, namun, ada sekitar 10 loop seperti itu dalam proyek.
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 , diambil oleh fungsi, segera diubah dengan nilai lain. Sangat mencurigakan. Selain itu, kode tersebut berisi beberapa lusin tempat seperti itu, jadi sulit untuk memutuskan apakah ini kesalahan atau tidak. Saya menemukan kode yang sama di proyek besar lainnya, kode seperti itu benar di sana, 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
Analisator telah mendeteksi kelebihan fungsi
headerData yang tidak valid. Fungsi memiliki nilai default yang berbeda dari parameter
peran . Ini dapat menyebabkan perilaku yang salah, bukan yang diharapkan oleh seorang 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 menurut standar bahasa.
Kemungkinan besar, kode yang benar harus seperti ini:
if (bitshift == 64) available_bits = 0; else available_bits = G_GUINT64_CONSTANT(0xFFFFFFFFFFFFFFFF) >> bitshift;
Kesimpulan
Tampaknya ulasan ini menunjukkan beberapa kesalahan, tetapi dalam laporan lengkap kasus yang dipertimbangkan berulang puluhan dan ratusan kali. Selain itu, ulasan peringatan PVS-Studio bersifat demonstratif. Mereka mewakili kontribusi terhadap kualitas proyek sumber terbuka, tetapi pemeriksaan satu kali adalah yang paling tidak efisien dalam hal metodologi analisis statis.
Anda bisa mendapatkan dan menganalisis sendiri laporan lengkapnya. Untuk melakukan ini, Anda hanya perlu
mengunduh dan menjalankan alat analisa PVS-Studio.