Cara menembak diri Anda di kaki di C dan C ++. Koleksi Resep OS Haiku

Kisah pertemuan analisa statis PVS-Studio dengan kode sistem operasi Haiku kembali ke tahun 2015 yang jauh. Itu adalah eksperimen yang menarik dan pengalaman yang berguna bagi tim dari kedua proyek. Mengapa harus bereksperimen? Tidak ada analisa untuk Linux saat itu dan tidak akan ada satu setengah tahun lagi. Tetapi pekerjaan para penggemar tim kami dihargai: kami bertemu dengan para pengembang Haiku dan meningkatkan kualitas kode, mengisi ulang database dengan kesalahan programmer yang jarang terjadi, dan menyempurnakan penganalisa. Memeriksa kode Haiku untuk kesalahan cepat dan mudah sekarang.

Gambar 3


Pendahuluan


Pahlawan sejarah kita adalah sistem operasi Haiku open-source dan analisa statis PVS-Studio untuk C, C ++, C # dan Java. Ketika 4,5 tahun yang lalu kami mulai menganalisis proyek, kami hanya perlu bekerja dengan file executable analyzer yang dikompilasi. Seluruh infrastruktur untuk parsing parameter kompilasi, mulai preprocessor, analisis paralelisasi, dll. diambil dari utilitas UI Pemantau Kompiler dalam C #, yang portingnya sebagian ke platform Mono untuk dijalankan di Linux. Proyek Haiku sendiri dibangun menggunakan cross-compiler di bawah berbagai sistem operasi, kecuali Windows. Sekali lagi, saya ingin mencatat kenyamanan dan kelengkapan dokumentasi perakitan Haiku, dan juga berterima kasih kepada para pengembang Haiku atas bantuan mereka dalam membangun proyek.

Sekarang analisis jauh lebih mudah. Daftar semua perintah untuk membangun dan menganalisis proyek terlihat seperti ini:

cd /opt git clone https://review.haiku-os.org/buildtools git clone https://review.haiku-os.org/haiku cd ./haiku mkdir generated.x86_64; cd generated.x86_64 ../configure --distro-compatibility official -j12 \ --build-cross-tools x86_64 ../../buildtools cd ../../buildtools/jam make all cd /opt/haiku/generated.x86_64 pvs-studio-analyzer trace -- /opt/buildtools/jam/bin.linuxx86/jam \ -q -j1 @nightly-anyboot pvs-studio-analyzer analyze -l /mnt/svn/PVS-Studio.lic -r /opt/haiku \ -C x86_64-unknown-haiku-gcc -o /opt/haiku/haiku.log -j12 

Omong-omong, analisis proyek dilakukan dalam wadah Docker. Baru-baru ini, kami telah menyiapkan dokumentasi baru tentang topik ini: Menjalankan PVS-Studio di Docker . Ini dapat sangat menyederhanakan penggunaan teknik analisis proyek statis untuk beberapa perusahaan.

Variabel tidak diinisialisasi


V614 Variabel tidak diinisialisasi 'rval' digunakan. fetch.c 1727

 int auto_fetch(int argc, char *argv[]) { volatile int argpos; int rval; // <= argpos = 0; if (sigsetjmp(toplevel, 1)) { if (connected) disconnect(0, NULL); if (rval > 0) // <= rval = argpos + 1; return (rval); } .... } 

Variabel rval tidak diinisialisasi ketika dideklarasikan, jadi membandingkannya dengan nilai nol akan menghasilkan hasil yang tidak ditentukan. Jika keadaan gagal, nilai variabel rval yang tidak ditentukan dapat menjadi nilai balik fungsi auto_fetch .

V614 'Res' pointer tidak diinisialisasi digunakan. commands.c 2873

 struct addrinfo { int ai_flags; int ai_family; int ai_socktype; int ai_protocol; socklen_t ai_addrlen; char *ai_canonname; struct sockaddr *ai_addr; struct addrinfo *ai_next; }; static int sourceroute(struct addrinfo *ai, char *arg, char **cpp, int *lenp, int *protop, int *optp) { static char buf[1024 + ALIGNBYTES]; char *cp, *cp2, *lsrp, *ep; struct sockaddr_in *_sin; #ifdef INET6 struct sockaddr_in6 *sin6; struct ip6_rthdr *rth; #endif struct addrinfo hints, *res; // <= int error; char c; if (cpp == NULL || lenp == NULL) return -1; if (*cpp != NULL) { switch (res->ai_family) { // <= case AF_INET: if (*lenp < 7) return -1; break; .... } } .... } 

Kasus serupa menggunakan variabel tidak diinisialisasi, hanya di sini adalah pointer pointer diinisialisasi.

Pointer V506 ke variabel lokal 'dinormalisasi' disimpan di luar ruang lingkup variabel ini. Pointer seperti itu akan menjadi tidak valid. TextView.cpp 5596

 void BTextView::_ApplyStyleRange(...., const BFont* font, ....) { if (font != NULL) { BFont normalized = *font; _NormalizeFont(&normalized); font = &normalized; } .... fStyles->SetStyleRange(fromOffset, toOffset, fText->Length(), mode, font, color); } 

Mungkin, programmer perlu menormalkan objek melalui variabel perantara. Tapi sekarang, di penunjuk font , penunjuk ke objek sementara yang dinormalisasi disimpan , yang akan dihancurkan setelah meninggalkan ruang lingkup di mana objek sementara ini dibuat.

V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 27

 int8 BUnicodeChar::Type(uint32 c) { BUnicodeChar(); return u_charType(c); } 

Kesalahan yang sangat umum di antara programmer C ++ adalah menggunakan panggilan konstruktor yang seharusnya untuk menginisialisasi / nol bidang kelas. Dalam hal ini, tidak ada modifikasi bidang kelas, tetapi objek baru yang tidak disebutkan namanya dari kelas ini dibuat, yang segera dihancurkan. Sayangnya, ada banyak tempat seperti itu:

  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 37
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 49
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 58
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 67
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 77
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 89
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 103
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 115
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 126
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 142
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 152
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 163
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 186
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 196
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 206
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 214
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 222
  • V603 Objek telah dibuat tetapi tidak sedang digunakan. Jika Anda ingin memanggil konstruktor, 'this-> BUnicodeChar :: BUnicodeChar (....)' harus digunakan. UnicodeChar.cpp 230

V670 Anggota kelas 'fPatternHandler' yang tidak diinisialisasi digunakan untuk menginisialisasi anggota 'fInternal'. Ingat bahwa anggota diinisialisasi dalam urutan deklarasi mereka di dalam kelas. Painter.cpp 184

 Painter::Painter() : fInternal(fPatternHandler), .... fPatternHandler(), .... { .... }; class Painter { .... private: mutable PainterAggInterface fInternal; // line 336 bool fSubpixelPrecise : 1; bool fValidClipping : 1; bool fDrawingText : 1; bool fAttached : 1; bool fIdentityTransform : 1; Transformable fTransform; float fPenSize; const BRegion* fClippingRegion; drawing_mode fDrawingMode; source_alpha fAlphaSrcMode; alpha_function fAlphaFncMode; cap_mode fLineCapMode; join_mode fLineJoinMode; float fMiterLimit; PatternHandler fPatternHandler; // line 355 mutable AGGTextRenderer fTextRenderer; }; 

Contoh lain inisialisasi yang salah. Bidang kelas diinisialisasi dalam urutan mereka dideklarasikan di kelas itu sendiri. Dalam contoh ini, bidang fInternal akan diinisialisasi pertama menggunakan nilai yang tidak diinisialisasi fPatternHandler .

#Define mencurigakan


V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. subr_gtaskqueue.c 191

 #define TQ_LOCK(tq) \ do { \ if ((tq)->tq_spin) \ mtx_lock_spin(&(tq)->tq_mutex); \ else \ mtx_lock(&(tq)->tq_mutex); \ } while (0) #define TQ_ASSERT_LOCKED(tq) mtx_assert(&(tq)->tq_mutex, MA_OWNED) #define TQ_UNLOCK(tq) \ do { \ if ((tq)->tq_spin) \ mtx_unlock_spin(&(tq)->tq_mutex); \ else \ mtx_unlock(&(tq)->tq_mutex); \ } while (0) void grouptask_block(struct grouptask *grouptask) { .... TQ_LOCK(queue); gtask->ta_flags |= TASK_NOENQUEUE; gtaskqueue_drain_locked(queue, gtask); TQ_UNLOCK(queue); } 

Cuplikan kode tidak terlihat mencurigakan hingga Anda melihat hasil preprocessor:

 void grouptask_block(struct grouptask *grouptask) { .... do { if ((queue)->tq_spin) mtx_lock(&(queue)->tq_mutex); else mtx_lock(&(queue)->tq_mutex); } while (0); gtask->ta_flags |= 0x4; gtaskqueue_drain_locked(queue, gtask); do { if ((queue)->tq_spin) mtx_unlock(&(queue)->tq_mutex); else mtx_unlock(&(queue)->tq_mutex); } while (0); } 

Alat analisis benar-benar - cabang if dan else identik. Tetapi kemana perginya fungsi mtx_lock_spin dan mtx_unlock_spin ? Makro TQ_LOCK , TQ_UNLOCK dan fungsi grouptask_block dideklarasikan dalam file yang sama dan hampir berdampingan, namun ada substitusi di suatu tempat.

Pencarian pada isi file hanya ditemukan mutex.h dengan konten berikut:

 /* on FreeBSD these are different functions */ #define mtx_lock_spin(x) mtx_lock(x) #define mtx_unlock_spin(x) mtx_unlock(x) 

Apakah substitusi seperti itu benar atau tidak, layak untuk memeriksa pengembang proyek. Saya memeriksa proyek di Linux, dan penggantian seperti itu tampak mencurigakan bagi saya.

Kesalahan dengan fungsi bebas


V575 Penunjuk nol dilewatkan ke fungsi 'bebas'. Periksa argumen pertama. setmime.cpp 727

 void MimeType::_PurgeProperties() { fShort.Truncate(0); fLong.Truncate(0); fPrefApp.Truncate(0); fPrefAppSig.Truncate(0); fSniffRule.Truncate(0); delete fSmallIcon; fSmallIcon = NULL; delete fBigIcon; fBigIcon = NULL; fVectorIcon = NULL; // <= free(fVectorIcon); // <= fExtensions.clear(); fAttributes.clear(); } 

Anda dapat melewatkan pointer nol ke fungsi bebas , tetapi entri seperti itu jelas mencurigakan. Jadi, penganalisa menemukan garis kode yang membingungkan. Pertama, Anda harus membebaskan memori menggunakan pointer fVectorIcon , dan hanya kemudian mengaturnya ke NULL .

V575 Penunjuk nol dilewatkan ke fungsi 'bebas'. Periksa argumen pertama. driver_settings.cpp 461

 static settings_handle * load_driver_settings_from_file(int file, const char *driverName) { .... handle = new_settings(text, driverName); if (handle != NULL) { // everything went fine! return handle; } free(handle); // <= .... } 

Contoh lain dari pengoper null pointer secara eksplisit ke fungsi bebas . Baris ini dapat dihapus, karena setelah berhasil menerima pointer, fungsi keluar.

V575 Penunjuk nol dilewatkan ke fungsi 'bebas'. Periksa argumen pertama. PackageFileHeapWriter.cpp 166

 void* _GetBuffer() { .... void* buffer = malloc(fBufferSize); if (buffer == NULL && !fBuffers.AddItem(buffer)) { free(buffer); throw std::bad_alloc(); } return buffer; } 

Ini kesalahan. Alih-alih operator &&, operator || harus digunakan. Hanya dalam kasus ini pengecualian akan dilemparkan std :: bad_alloc () jika tidak memungkinkan untuk mengalokasikan memori menggunakan fungsi malloc .

Kesalahan dengan operator hapus


V611 Memori dialokasikan menggunakan operator 'T baru] tetapi dirilis menggunakan operator' hapus '. Pertimbangkan untuk memeriksa kode ini. Mungkin lebih baik menggunakan 'delete [] fMsg;'. Err.cpp 65

 class Err { public: .... private: char *fMsg; ssize_t fPos; }; void Err::Unset() { delete fMsg; // <= fMsg = __null; fPos = -1; } void Err::SetMsg(const char *msg) { if (fMsg) { delete fMsg; // <= fMsg = __null; } if (msg) { fMsg = new(std::nothrow) char[strlen(msg)+1]; // <= if (fMsg) strcpy(fMsg, msg); } } 

Pointer fMsg digunakan untuk mengalokasikan memori untuk menyimpan berbagai karakter, dan operator hapus digunakan untuk mengosongkan memori, alih-alih menghapus [] .

V611 Memori dialokasikan menggunakan operator 'baru' tetapi dirilis menggunakan fungsi 'bebas'. Pertimbangkan untuk memeriksa logika operasi di belakang variabel 'wrapperPool'. vm_page.cpp 3080

 status_t vm_page_write_modified_page_range(....) { .... PageWriteWrapper* wrapperPool = new(malloc_flags(allocationFlags)) PageWriteWrapper[maxPages + 1]; PageWriteWrapper** wrappers = new(malloc_flags(allocationFlags)) PageWriteWrapper*[maxPages]; if (wrapperPool == NULL || wrappers == NULL) { free(wrapperPool); // <= free(wrappers); // <= wrapperPool = stackWrappersPool; wrappers = stackWrappers; maxPages = 1; } .... } 

Di sini malloc_flags adalah fungsi yang membuat malloc . Dan kemudian penempatan-baru membangun objek di sini. Dan karena kelas PageWriteWrapper diimplementasikan sebagai berikut:

 class PageWriteWrapper { public: PageWriteWrapper(); ~PageWriteWrapper(); void SetTo(vm_page* page); bool Done(status_t result); private: vm_page* fPage; struct VMCache* fCache; bool fIsActive; }; PageWriteWrapper::PageWriteWrapper() : fIsActive(false) { } PageWriteWrapper::~PageWriteWrapper() { if (fIsActive) panic("page write wrapper going out of scope but isn't completed"); } 

maka objek destruktor dari kelas ini tidak akan dipanggil karena penggunaan fungsi bebas untuk membebaskan memori.

V611 Memori dialokasikan menggunakan operator 'T baru] tetapi dirilis menggunakan operator' hapus '. Pertimbangkan untuk memeriksa kode ini. Mungkin lebih baik menggunakan 'delete [] fOutBuffer;'. Periksa baris: 26, 45. PCL6Rasterizer.h 26

 class PCL6Rasterizer : public Rasterizer { public: .... ~PCL6Rasterizer() { delete fOutBuffer; fOutBuffer = NULL; } .... virtual void InitializeBuffer() { fOutBuffer = new uchar[fOutBufferSize]; } private: uchar* fOutBuffer; int fOutBufferSize; }; 

Menggunakan operator hapus alih-alih menghapus [] adalah kesalahan yang sangat umum. Cara termudah untuk membuat kesalahan saat menulis kelas adalah Kode destruktor sering terletak jauh dari lokasi alokasi memori. Di sini, programmer secara tidak benar membebaskan memori dalam destruktor oleh pointer fOutBuffer .

V772 Memanggil operator 'hapus' untuk pointer kosong akan menyebabkan perilaku tidak terdefinisi. Hashtable.cpp 207

 void Hashtable::MakeEmpty(int8 keyMode,int8 valueMode) { .... for (entry = fTable[index]; entry; entry = next) { switch (keyMode) { case HASH_EMPTY_DELETE: // TODO: destructors are not called! delete (void*)entry->key; break; case HASH_EMPTY_FREE: free((void*)entry->key); break; } switch (valueMode) { case HASH_EMPTY_DELETE: // TODO: destructors are not called! delete entry->value; break; case HASH_EMPTY_FREE: free(entry->value); break; } next = entry->next; delete entry; } .... } 

Selain pilihan yang salah antara hapus / hapus [] dan gratis , Anda dapat menambahkan perilaku tidak terdefinisi ke program dengan cara lain - coba hapus memori dengan pointer ke tipe yang tidak ditentukan (batal *) .

Fungsi tanpa nilai kembali


Fungsi V591 Non-void harus mengembalikan nilai. Referenceable.h 228

 BReference& operator=(const BReference<const Type>& other) { fReference = other.fReference; } 

Operator tugas yang ditimpa tidak memiliki nilai pengembalian yang cukup. Dalam hal ini, operator akan mengembalikan nilai acak, yang dapat menyebabkan kesalahan aneh.

Masalah serupa di cuplikan kode lain dari kelas ini:

  • Fungsi V591 Non-void harus mengembalikan nilai. Referenceable.h 233
  • Fungsi V591 Non-void harus mengembalikan nilai. Referenceable.h 239

Fungsi V591 Non-void harus mengembalikan nilai. main.c 1010

 void errx(int, const char *, ...) ; char * getoptionvalue(const char *name) { struct option *c; if (name == NULL) errx(1, "getoptionvalue() invoked with NULL name"); c = getoption(name); if (c != NULL) return (c->value); errx(1, "getoptionvalue() invoked with unknown option '%s'", name); /* NOTREACHED */ } 

Komentar pengguna NOTREACHED di sini tidak ada artinya. Untuk menulis kode dengan benar untuk skenario seperti itu, Anda perlu menandai fungsi seperti noreturn. Ada atribut noreturn untuk ini: standar dan khusus kompiler. Pertama-tama, atribut tersebut diperhitungkan oleh kompiler untuk pembuatan kode yang benar atau pemberitahuan beberapa jenis kesalahan dengan bantuan peringatan. Berbagai alat analisis statis juga mempertimbangkan atribut untuk meningkatkan kualitas analisis.

Penanganan pengecualian


V596 Objek telah dibuat tetapi tidak sedang digunakan. Kata kunci 'lempar' bisa hilang: lempar ParseException (FOO); Response.cpp 659

 size_t Response::ExtractNumber(BDataIO& stream) { BString string = ExtractString(stream); const char* end; size_t number = strtoul(string.String(), (char**)&end, 10); if (end == NULL || end[0] != '\0') ParseException("Invalid number!"); return number; } 

Di sini, kata kunci lemparan tidak sengaja terlupakan. Dengan demikian, ParseException tidak akan dilempar , dan objek dari kelas ini hanya akan dihancurkan ketika meninggalkan ruang lingkup. Setelah itu fungsi akan melanjutkan kerjanya seolah-olah tidak ada yang terjadi, seolah-olah nomor yang benar dimasukkan.

V1022 Pengecualian dilemparkan oleh pointer. Pertimbangkan untuk melemparkannya dengan nilai saja. gensyscallinfos.cpp 316

 int main(int argc, char** argv) { try { return Main().Run(argc, argv); } catch (Exception& exception) { // <= fprintf(stderr, "%s\n", exception.what()); return 1; } } int Run(int argc, char** argv) { .... _ParseSyscalls(argv[1]); .... } void _ParseSyscalls(const char* filename) { ifstream file(filename, ifstream::in); if (!file.is_open()) throw new IOException(string("Failed to open '") + filename + "'."); // <= .... } 

Analyzer mendeteksi IOException yang dilemparkan oleh pointer. Melempar pointer menyebabkan pengecualian tidak ditangkap, sehingga pengecualian akhirnya ditangkap oleh referensi. Juga, menggunakan pointer memaksa pencegat untuk memanggil operator hapus untuk menghancurkan objek yang dibuat, yang juga tidak dilakukan.

Dua area masalah lagi dari kode:

  • V1022 Pengecualian dilemparkan oleh pointer. Pertimbangkan untuk melemparkannya dengan nilai saja. gensyscallinfos.cpp 347
  • V1022 Pengecualian dilemparkan oleh pointer. Pertimbangkan untuk melemparkannya dengan nilai saja. gensyscallinfos.cpp 413

Keselamatan formal


V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk flush objek 'f_key'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. dst_api.c 1018

 #ifndef SAFE_FREE #define SAFE_FREE(a) \ do{if(a != NULL){memset(a,0, sizeof(*a)); free(a); a=NULL;}} while (0) .... #endif DST_KEY * dst_free_key(DST_KEY *f_key) { if (f_key == NULL) return (f_key); if (f_key->dk_func && f_key->dk_func->destroy) f_key->dk_KEY_struct = f_key->dk_func->destroy(f_key->dk_KEY_struct); else { EREPORT(("dst_free_key(): Unknown key alg %d\n", f_key->dk_alg)); } if (f_key->dk_KEY_struct) { free(f_key->dk_KEY_struct); f_key->dk_KEY_struct = NULL; } if (f_key->dk_key_name) SAFE_FREE(f_key->dk_key_name); SAFE_FREE(f_key); return (NULL); } 

Penganalisis mendeteksi kode mencurigakan yang dirancang untuk membersihkan data pribadi dengan aman. Sayangnya, makro SAFE_FREE , yang diperluas menjadi memset , panggilan gratis , dan penugasan NULL , tidak membuat kode lebih aman, karena semua ini dihapus oleh kompiler selama optimasi O2 .

By the way, ini tidak seperti CWE-14 : Penghapusan Kompiler Kode ke Hapus Buffer.

Seluruh daftar tempat di mana buffer sebenarnya tidak dibersihkan:

  • V597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'encoded_block'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. dst_api.c 446
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'key_st'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. dst_api.c 685
  • V597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'in_buff'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. dst_api.c 916
  • V597 Kompiler dapat menghapus panggilan fungsi 'memset', yang digunakan untuk menyiram objek 'ce'. Fungsi memset_s () harus digunakan untuk menghapus data pribadi. fs_cache.c 1078

Perbandingan yang Tidak Ditandatangani


Ekspresi V547 'tersisa <0' selalu salah. Nilai tipe yang tidak ditandatangani tidak pernah <0. DwarfFile.cpp 1947

 status_t DwarfFile::_UnwindCallFrame(....) { .... uint64 remaining = lengthOffset + length - dataReader.Offset(); if (remaining < 0) return B_BAD_DATA; .... } 

Penganalisa menemukan perbandingan eksplisit dari variabel yang tidak ditandatangani dengan nilai negatif. Mungkin Anda harus membandingkan variabel yang tersisa dengan hanya nol atau menerapkan pemeriksaan overflow.

V547 Ekspresi 'sleep ((unsigned) secs) <0' selalu salah. Nilai tipe yang tidak ditandatangani tidak pernah <0. Misc.cpp 56

 status_t snooze(bigtime_t amount) { if (amount <= 0) return B_OK; int64 secs = amount / 1000000LL; int64 usecs = amount % 1000000LL; if (secs > 0) { if (sleep((unsigned)secs) < 0) // <= return errno; } if (usecs > 0) { if (usleep((useconds_t)usecs) < 0) return errno; } return B_OK; } 

Untuk memahami apa kesalahannya, mari kita beralih ke tanda tangan fungsi sleep and usleep :

 extern unsigned int sleep (unsigned int __seconds); extern int usleep (__useconds_t __useconds); 

Seperti yang bisa kita lihat, fungsi sleep mengembalikan nilai dari tipe yang tidak ditandatangani dan penggunaannya dalam kode salah.

Pointer berbahaya


V774 Pointer 'perangkat' digunakan setelah memori dilepaskan. xhci.cpp 1572

 void XHCI::FreeDevice(Device *device) { uint8 slot = fPortSlots[device->HubPort()]; TRACE("FreeDevice() port %d slot %d\n", device->HubPort(), slot); // Delete the device first, so it cleans up its pipes and tells us // what we need to destroy before we tear down our internal state. delete device; DisableSlot(slot); fDcba->baseAddress[slot] = 0; fPortSlots[device->HubPort()] = 0; // <= delete_area(fDevices[slot].trb_area); delete_area(fDevices[slot].input_ctx_area); delete_area(fDevices[slot].device_ctx_area); memset(&fDevices[slot], 0, sizeof(xhci_device)); fDevices[slot].state = XHCI_STATE_DISABLED; } 

Objek perangkat dihapus oleh operator hapus . Ini adalah tindakan logis untuk fungsi yang disebut FreeDevice . Tetapi, untuk beberapa alasan, untuk melepaskan sumber daya lain dalam kode, ada lagi daya tarik ke objek yang sudah dihapus.

Kode semacam itu sangat berbahaya dan terjadi di beberapa tempat lain:

  • V774 Pointer 'mandiri' digunakan setelah memori dilepaskan. TranslatorRoster.cpp 884
  • V774 Pointer 'string' digunakan setelah memori dilepaskan. RemoteView.cpp 1269
  • V774 Pointer 'bs' digunakan setelah memori dilepaskan. mkntfs.c 4291
  • V774 Pointer 'bs' digunakan setelah memori dilepaskan. mkntfs.c 4308
  • V774 Pointer 'al' digunakan setelah memori dialokasikan kembali. inode.c 1155

V522 Dereferencing dari 'data' pointer nol mungkin terjadi. Pointer nol dilewatkan ke fungsi 'malo_hal_send_helper'. Periksa argumen ketiga. Periksa baris: 350, 394. if_malohal.c 350

 static int malo_hal_fwload_helper(struct malo_hal *mh, char *helper) { .... /* tell the card we're done and... */ error = malo_hal_send_helper(mh, 0, NULL, 0, MALO_NOWAIT); // <= NULL .... } static int malo_hal_send_helper(struct malo_hal *mh, int bsize, const void *data, size_t dsize, int waitfor) { mh->mh_cmdbuf[0] = htole16(MALO_HOSTCMD_CODE_DNLD); mh->mh_cmdbuf[1] = htole16(bsize); memcpy(&mh->mh_cmdbuf[4], data , dsize); // <= data .... } 

Analisis antar prosedur mengungkapkan situasi di mana NULL diteruskan ke fungsi dan penunjuk data dengan nilai ini kemudian ditinjau kembali dalam fungsi memcpy .

V773 Fungsi itu keluar tanpa melepaskan pointer 'inputFileFile'. Kebocoran memori dimungkinkan. command_recompress.cpp 119

 int command_recompress(int argc, const char* const* argv) { .... BFile* inputFileFile = new BFile; error = inputFileFile->SetTo(inputPackageFileName, O_RDONLY); if (error != B_OK) { fprintf(stderr, "Error: Failed to open input file \"%s\": %s\n", inputPackageFileName, strerror(error)); return 1; } inputFile = inputFileFile; .... } 

PVS-Studio dapat mendeteksi kebocoran memori . Di sini, memori tidak dibebaskan untuk inputFileFile jika terjadi beberapa jenis kesalahan. Seseorang percaya bahwa jika terjadi kesalahan, Anda tidak dapat repot-repot dengan membebaskan memori - program masih akan berakhir. Tapi ini tidak selalu terjadi. Menangani kesalahan dengan benar dan terus bekerja - suatu persyaratan untuk banyak program.

V595 Pointer 'fReply' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 49, 52. BalasBuilder.cpp 49

 RPC::CallbackReply* ReplyBuilder::Reply() { fReply->Stream().InsertUInt(fStatusPosition, _HaikuErrorToNFS4(fStatus)); fReply->Stream().InsertUInt(fOpCountPosition, fOpCount); if (fReply == NULL || fReply->Stream().Error() == B_OK) return fReply; else return NULL; } 

Seberapa sering pengembang menunjuk petunjuk sebelum memeriksa. Diagnostik V595 hampir selalu menjadi pemimpin dalam jumlah peringatan dalam suatu proyek. Penggunaan pointer fReply yang berbahaya dalam bagian kode ini.

V595 Pointer 'mq' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 782, 786. oce_queue.c 782

 static void oce_mq_free(struct oce_mq *mq) { POCE_SOFTC sc = (POCE_SOFTC) mq->parent; struct oce_mbx mbx; struct mbx_destroy_common_mq *fwcmd; if (!mq) return; .... } 

Contoh serupa. Pointer mg direferensikan beberapa baris lebih awal daripada diperiksa untuk nilai nol. Ada banyak tempat serupa dalam proyek ini. Di beberapa tempat, menggunakan dan memeriksa pointer jauh dari satu sama lain, jadi dua contoh disertakan dalam artikel. Selebihnya akan dapat melihat pengembang dalam laporan analisa lengkap.

Kesalahan lain-lain


V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 101

 static void dump_acpi_namespace(acpi_ns_device_info *device, char *root, int indenting) { char output[320]; char tabs[255] = ""; .... strlcat(tabs, "|--- ", sizeof(tabs)); .... while (....) { uint32 type = device->acpi->get_object_type(result); snprintf(output, sizeof(output), "%s%s", tabs, result + depth); switch(type) { case ACPI_TYPE_INTEGER: strncat(output, " INTEGER", sizeof(output)); break; case ACPI_TYPE_STRING: strncat(output, " STRING", sizeof(output)); break; .... } .... } .... } 

Perbedaan antara fungsi strlcat dan strncat tidak sepenuhnya jelas bagi orang yang baru mengenal deskripsi fungsi-fungsi ini. Fungsi strlcat mengambil ukuran seluruh buffer sebagai argumen ketiga, dan fungsi strncat mengambil ukuran ruang kosong dalam buffer , yang mengharuskan penghitungan nilai yang diinginkan sebelum memanggil fungsi. Namun pengembang sering lupa atau tidak mengetahuinya. Melewati fungsi strncat ke ukuran seluruh buffer dapat menyebabkan buffer overflow, karena fungsi akan menganggap nilai ini sebagai jumlah karakter yang diizinkan untuk disalin. Fungsi strlcat tidak memiliki masalah ini, tetapi agar berfungsi dengan benar, perlu mentransfer saluran yang berakhir di terminal nol.

Seluruh daftar tempat berbahaya dengan garis:

  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 104
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 107
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 110
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 113
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 118
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 119
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 120
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 123
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 126
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 129
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 132
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 135
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 138
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 141
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 144
  • V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 283
  • V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 284
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'fitur_string'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. VirtioDevice.cpp 285

V792 Fungsi 'SetDecoratorSettings' terletak di sebelah kanan operator '|' akan dipanggil terlepas dari nilai operan kiri. Mungkin, lebih baik menggunakan '||'. DesktopListener.cpp 324

 class DesktopListener : public DoublyLinkedListLinkImpl<DesktopListener> { public: .... virtual bool SetDecoratorSettings(Window* window, const BMessage& settings) = 0; .... }; bool DesktopObservable::SetDecoratorSettings(Window* window, const BMessage& settings) { if (fWeAreInvoking) return false; InvokeGuard invokeGuard(fWeAreInvoking); bool changed = false; for (DesktopListener* listener = fDesktopListenerList.First(); listener != NULL; listener = fDesktopListenerList.GetNext(listener)) changed = changed | listener->SetDecoratorSettings(window, settings); return changed; } 

Kemungkinan besar, operator '|' dan '||'. Kesalahan seperti itu menyebabkan panggilan yang tidak perlu ke fungsi SetDecoratorSettings .

V627 Pertimbangkan untuk memeriksa ekspresi. Argumen sizeof () adalah makro yang diperluas ke angka. device.c 72

 #define PCI_line_size 0x0c /* (1 byte) cache line size in 32 bit words */ static status_t wb840_open(const char* name, uint32 flags, void** cookie) { .... data->wb_cachesize = gPci->read_pci_config(data->pciInfo->bus, data->pciInfo->device, data->pciInfo->function, PCI_line_size, sizeof(PCI_line_size)) & 0xff; .... } 

Melewati nilai 0x0c ke ukuran operator terlihat mencurigakan. Mungkin Anda harus menghitung ukuran beberapa objek, misalnya, data . V562 Aneh membandingkan nilai tipe bool dengan nilai 18: 0x12 == IsProfessionalSpdif (). CEchoGals_mixer.cpp 533



 typedef bool BOOL; virtual BOOL IsProfessionalSpdif() { ... } #define ECHOSTATUS_DSP_DEAD 0x12 ECHOSTATUS CEchoGals::ProcessMixerFunction(....) { .... if ( ECHOSTATUS_DSP_DEAD == IsProfessionalSpdif() ) // <= { Status = ECHOSTATUS_DSP_DEAD; } else { pMixerFunction->Data.bProfSpdif = IsProfessionalSpdif(); } .... } 

Fungsi IsProfessionalSpdif mengembalikan nilai tipe bool , sedangkan dalam kondisi hasil fungsi dibandingkan dengan angka 0x12 .

Kesimpulan


Kami melewatkan rilis beta Haiku pertama musim gugur yang lalu, karena sibuk merilis PVS-Studio untuk bahasa Jawa. Tetapi sifat kesalahan programmer adalah sedemikian sehingga mereka tidak pergi ke mana pun kecuali Anda mencarinya dan tidak memperhatikan kualitas kode sama sekali. Pengembang mencoba menggunakan Coverity Scan , tetapi analisis terakhir hampir dua tahun lalu. Ini akan mengganggu pengguna Haiku. Meskipun analisis menggunakan Cakupan didirikan pada tahun 2014, ini tidak menghentikan kami dari menulis dua artikel besar dengan ulasan kesalahan pada tahun 2015 ( bagian 1 , bagian 2 ).

Mereka yang telah membaca sampai akhir menunggu tinjauan lain dari kode Haiku yang tidak kalah volumenya dengan jenis kesalahan baru. Laporan analisa lengkap akan dikirim ke pengembang sebelum menerbitkan ulasan kode, sehingga beberapa kesalahan dapat diperbaiki. Untuk menghabiskan waktu di antara publikasi, saya sarankan mengunduh dan mencoba PVS-Studio pada proyek Anda.

Ingin mencoba Haiku dan memiliki pertanyaan? Pengembang Haiku mengundang Anda ke saluran telegram .



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Svyatoslav Razmyslov. Cara menembak diri Anda di kaki di C dan C ++. Haiku OS Cookbook

Source: https://habr.com/ru/post/id461255/


All Articles