Geekly articles weekly

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

Kisah tentang bagaimana penganalisa statis PVS-Studio dan kode OS Haiku bertemu kembali ke tahun 2015. Itu adalah eksperimen yang menarik dan pengalaman yang berguna untuk tim dari kedua proyek. Kenapa harus bereksperimen? Pada saat itu, kami tidak memiliki analisa untuk Linux dan kami tidak akan memilikinya untuk satu setengah tahun lagi. Bagaimanapun, upaya para penggemar dari tim kami telah dihargai: kami berkumpul dengan pengembang Haiku dan meningkatkan kualitas kode, memperluas basis kesalahan kami dengan bug langka yang dibuat oleh pengembang dan menyempurnakan penganalisis. Sekarang Anda dapat memeriksa kode Haiku untuk kesalahan dengan mudah dan cepat.
Gambar 1


Pendahuluan


Temui karakter utama dari cerita kami - Haiku dengan kode sumber terbuka dan penganalisa statis PVS-Studio untuk C, C ++, C # dan Java. Ketika kami menggali analisis proyek 4,5 tahun yang lalu, kami hanya harus berurusan dengan file penganalisa yang dapat dieksekusi yang dikompilasi. Semua infrastruktur untuk parsing parameter kompiler, menjalankan preprosesor, sejajar dengan analisis dan seterusnya diambil dari utilitas Compiler Monitoring UI , ditulis dalam C #. Utilitas itu porting sebagian ke platform Mono untuk dijalankan di Linux. Proyek Haiku dibangun menggunakan kompiler silang di bawah berbagai OS, kecuali Windows. Sekali lagi, saya ingin menyebutkan kenyamanan dan kelengkapan dokumentasi terkait dengan gedung Haiku. Saya juga ingin mengucapkan terima kasih kepada pengembang Haiku atas bantuan mereka dalam membangun proyek.

Jauh lebih mudah untuk melakukan analisis sekarang. Berikut adalah daftar semua perintah untuk membangun dan menganalisis proyek:

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 dilaksanakan dalam wadah Docker. Baru-baru ini kami telah menyiapkan dokumentasi baru tentang topik ini: Menjalankan PVS-Studio di Docker . Ini dapat membuatnya sangat mudah bagi beberapa perusahaan untuk menerapkan teknik analisis statis untuk proyek mereka.

Variabel tidak diinisialisasi


V614 'rval' variabel tidak diinisialisasi 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 belum diinisialisasi pada deklarasi, jadi perbandingannya dengan nilai nol akan menghasilkan hasil yang tidak ditentukan. Jika keadaan gagal, nilai tidak pasti dari variabel rval dapat menjadi nilai kembali 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; .... } } .... }

Berikut adalah kasus serupa menggunakan variabel tidak diinisialisasi, kecuali bahwa res adalah pointer tidak diinisialisasi yang terjadi di sini.

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

Pemrogram mungkin perlu menormalkan objek menggunakan variabel perantara. Tapi sekarang pointer font berisi pointer ke objek yang dinormalisasi , yang akan dihapus setelah keluar dari ruang lingkup, tempat objek sementara 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 / membatalkan bidang kelas. Dalam hal ini, modifikasi bidang kelas tidak terjadi, tetapi objek baru yang tidak disebutkan namanya dari kelas ini dibuat dan kemudian segera dihancurkan. Sayangnya, ada banyak tempat seperti itu di proyek:

  • 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 deklarasi mereka di kelas itu sendiri. Dalam contoh ini, bidang fInternal akan menjadi yang pertama menginisialisasi menggunakan nilai fPatternHandler yang tidak diinisialisasi.

#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 ini tidak terlihat mencurigakan hingga Anda melihat hasil preprosesor:

 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 analisa benar-benar benar - jika dan cabang lainnya identik. Tetapi di manakah fungsi mtx_lock_spin dan mtx_unlock_spin ? Macros TQ_LOCK , TQ_UNLOCK dan fungsi grouptask_block dideklarasikan dalam satu file hampir di sebelah satu sama lain, tetapi meskipun demikian terjadi penggantian di suatu tempat di sini.

Pencarian melalui file hanya menghasilkan 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)

Pengembang proyek harus memeriksanya apakah penggantian semacam itu benar atau tidak. Saya memeriksa proyek ini 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 di fungsi bebas , tetapi penggunaan seperti itu pasti mencurigakan. Dengan demikian, penganalisa menemukan campuran garis kode. Pertama, pembuat kode harus melepaskan memori dengan pointer fVectorIcon , hanya setelah itu tetapkan 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); // <= .... }

Ini adalah contoh lain dari pengoperan null pointer secara eksplisit ke fungsi bebas . Baris ini dapat dihapus, saat fungsi keluar setelah mendapatkan pointer dengan sukses.

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; }

Seseorang membuat kesalahan di sini. Operator || harus digunakan alih-alih &&. Hanya dalam kasus ini pengecualian std :: bad_alloc () akan dilemparkan seandainya alokasi memori (menggunakan fungsi malloc ) gagal.

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 berbagai karakter. Operator hapus digunakan untuk melepaskan 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 memanggil malloc . Kemudian penempatan-baru membangun objek di sini. Karena kelas PageWriteWrapper diimplementasikan dengan cara 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"); }

penghancur objek dari kelas ini tidak akan dipanggil karena penggunaan fungsi bebas untuk melepaskan 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; };

Ini adalah kesalahan umum untuk menggunakan operator hapus alih-alih menghapus []. Cara termudah untuk membuat kesalahan saat menulis kelas, karena kode destruktor sering jauh dari lokasi memori. Di sini, programmer secara tidak benar membebaskan memori yang disimpan oleh pointer fOutBuffer di destructor.

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 juga dapat mengalami perilaku yang tidak terdefinisi saat mencoba menghapus memori dengan pointer ke tipe void (void *) .

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 penugasan yang kelebihan muatan tidak memiliki nilai balik. Dalam hal ini, operator akan mengembalikan nilai acak, yang dapat menyebabkan kesalahan aneh.

Berikut adalah masalah serupa di fragmen 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 tidak berarti apa-apa di sini. Anda perlu membuat anotasi fungsi sebagai noreturn agar dapat menulis kode dengan benar untuk skenario semacam itu. Untuk melakukan ini, ada atribut noreturn: standar dan khusus-kompiler. Pertama-tama, atribut-atribut ini diperhitungkan oleh kompiler untuk pembuatan kode yang tepat atau pemberitahuan tentang jenis bug tertentu yang menggunakan peringatan. Berbagai alat analisis statis juga memperhitungkan atribut akun untuk meningkatkan kualitas analisis.

Menangani 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; }

Pelemparan kata kunci tidak sengaja terlupakan di sini. Dengan demikian, pengecualian ParseException tidak akan dihasilkan sementara objek kelas ini akan dihancurkan saat keluar dari ruang lingkup. Setelah itu, fungsi akan terus bekerja seolah-olah tidak ada yang terjadi, seolah-olah nomor yang benar telah 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 + "'."); // <= .... }

Penganalisa mendeteksi pengecualian IOException yang dilemparkan oleh pointer. Melempar pointer mengarah pada fakta bahwa pengecualian tidak akan ditangkap. Jadi pengecualian akhirnya tertangkap oleh referensi. Selain itu, penggunaan pointer memaksa sisi penangkap untuk memanggil operator hapus untuk menghancurkan objek yang dibuat, yang belum dilakukan.

Beberapa fragmen kode lain yang bermasalah:

  • 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

Keamanan 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); }

Penganalisa telah mendeteksi kode yang mencurigakan, dimaksudkan untuk mengamankan data pribadi. Sayangnya, makro SAFE_FREE yang meluas ke memset , panggilan gratis , dan penugasan NULL tidak membuat kode lebih aman, karena semuanya dihapus oleh kompiler saat dioptimalkan dengan O2 .

By the way, itu tidak lain adalah CWE-14 : Penghapusan Kompiler Kode ke Hapus Buffer.

Berikut adalah daftar tempat, di mana pembersihan buffer tidak dilakukan pada kenyataannya:

  • 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 dengan variabel 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 unsigned dengan nilai negatif. Mungkin, seseorang harus membandingkan variabel yang tersisa hanya dengan nol atau mengimplementasikan pemeriksaan untuk 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 mendapatkan poin utama dari kesalahan, mari kita membahas tanda tangan fungsi sleep dan usleep :

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

Seperti yang dapat kita lihat, fungsi tidur mengembalikan nilai 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 dibuat oleh operator hapus . Ini cukup logis untuk fungsi FreeDevice . Tetapi, untuk beberapa alasan, untuk melepaskan sumber daya lain, objek yang sudah dihapus ditangani.

Kode tersebut sangat berbahaya dan dapat dipenuhi 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 prosedural mengungkapkan kasus ketika NULL diteruskan ke fungsi dan penunjuk data dengan nilai seperti itu akhirnya direferensikan 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 . Dalam contoh ini, jika terjadi kesalahan, memori tidak akan dirilis. Seseorang mungkin berpikir bahwa jika terjadi kesalahan Anda tidak perlu repot dengan rilis memori, karena program masih akan berakhir. Tetapi tidak selalu demikian. Ini adalah persyaratan bagi banyak program untuk menangani kesalahan dengan benar dan untuk terus bekerja.

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; }

Ini adalah kesalahan yang sangat umum untuk pointer dereferensi sebelum memeriksanya. Diagnostik V595 hampir selalu berlaku dalam jumlah peringatan dalam suatu proyek. Fragmen kode ini termasuk penggunaan pointer fReply yang berbahaya .

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 akan direferensikan beberapa baris lebih awal dari yang diperiksa untuk null. Ada banyak tempat serupa dalam proyek ini. Dalam beberapa cuplikan, penggunaan dan pemeriksaan pointer cukup jauh dari satu sama lain, jadi dalam artikel ini Anda hanya akan menemukan beberapa contoh seperti itu. Pengembang dipersilakan untuk memeriksa contoh lain dalam laporan analisa lengkap.

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 terlalu jelas bagi seseorang yang tidak terbiasa dengan deskripsi fungsi-fungsi ini. Fungsi strlcat mengharapkan ukuran seluruh buffer sebagai argumen ketiga sementara fungsi strncat - ukuran ruang kosong dalam buffer, yang mengharuskan mengevaluasi nilai yang diperlukan sebelum memanggil fungsi. Tetapi pengembang sering lupa atau tidak tahu tentang itu. Melewati seluruh ukuran buffer ke fungsi strncat dapat menyebabkan buffer overflow, karena fungsi akan mempertimbangkan nilai ini sebagai jumlah karakter yang dapat disalin. Fungsi strlcat tidak memiliki masalah seperti itu. Tetapi Anda harus melewatkan string, diakhiri dengan terminal null sehingga itu berfungsi dengan baik.

Berikut ini seluruh daftar tempat berbahaya dengan string:

  • 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'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 107
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 110
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 113
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 118
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 119
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 120
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 123
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 126
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 129
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 132
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 135
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 138
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 141
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'output'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. NamespaceDump.cpp 144
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'fitur_string'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. VirtioDevice.cpp 283
  • V645 Panggilan fungsi 'strncat' dapat menyebabkan buffer overflow 'fitur_string'. Batas tidak boleh berisi ukuran buffer, tetapi sejumlah karakter dapat menampungnya. 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, '|' dan '||' operator kacau. Kesalahan ini menyebabkan panggilan yang tidak perlu dari 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, penulis seharusnya mengevaluasi ukuran suatu 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 . Dengan demikian, hasil fungsi dibandingkan dengan angka 0x12 dalam kondisi.

Kesimpulan


Kami melewatkan rilis beta Haiku pertama musim gugur yang lalu, karena kami sibuk merilis PVS-Studio untuk Jawa. Namun sifat kesalahan pemrograman adalah sedemikian rupa sehingga tidak hilang jika Anda tidak mencarinya dan tidak memperhatikan kualitas kode. Pengembang proyek menggunakan Coverity Scan , tetapi yang terakhir hampir dua tahun yang lalu. Ini pasti mengecewakan pengguna Haiku. Meskipun analisis telah dikonfigurasi pada 2014 menggunakan Cakupan, itu tidak menghentikan kami dari menulis dua artikel panjang tentang tinjauan kesalahan pada tahun 2015 ( bagian 1 , bagian 2 )

Ulasan kesalahan Haiku lainnya akan segera keluar bagi mereka yang membaca posting ini sampai akhir. Laporan analisa lengkap akan dikirim ke pengembang sebelum memposting ulasan kesalahan ini, sehingga beberapa kesalahan mungkin diperbaiki pada saat Anda membaca ini. Untuk menghabiskan waktu di antara artikel, saya sarankan mengunduh dan mencoba PVS-Studio untuk proyek Anda.

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

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

More articles:


All Articles