Kami memeriksa kode sumber Android menggunakan PVS-Studio, atau tidak ada yang sempurna

A.
Pengembangan proyek kompleks besar tidak mungkin tanpa menggunakan metodologi dan alat pemrograman untuk membantu mengontrol kualitas kode. Pertama-tama, ini adalah standar pengkodean yang kompeten, ulasan kode, pengujian unit, penganalisa kode statis dan dinamis. Semua ini membantu mengidentifikasi cacat dalam kode pada tahap awal pengembangan. Artikel ini menunjukkan kemampuan analisa statis PVS-Studio untuk mendeteksi kesalahan dan kerentanan potensial dalam kode sistem operasi Android. Kami berharap bahwa artikel ini akan menarik pembaca ke metodologi analisis kode statis dan mereka ingin mengimplementasikannya dalam proses pengembangan proyek mereka sendiri.

Pendahuluan

Setahun telah berlalu sejak penulisan artikel besar tentang kesalahan pada sistem operasi Tizen, dan saya ingin sekali lagi melakukan penelitian yang sama menariknya dari beberapa sistem operasi. Pilihannya jatuh pada Android.

Kode sistem operasi Android berkualitas, teruji dengan baik. Selama pengembangan, setidaknya penganalisa kode statis Coverity digunakan, sebagaimana dibuktikan oleh komentar dari formulir:

/* Coverity: [FALSE-POSITIVE error] intended fall through */ /* Missing break statement between cases in switch statement */ /* fall through */ 

Secara umum, ini adalah proyek yang menarik, berkualitas tinggi, dan menemukan kesalahan di dalamnya merupakan tantangan bagi analisa statis PVS-Studio kami.

Saya berpikir bahwa hanya dengan volume artikel pembaca mengerti bahwa penganalisa PVS-Studio melakukan pekerjaan yang sangat baik dan menemukan banyak cacat dalam kode sistem operasi ini.

Enumerasi kelemahan umum

Dalam artikel ini Anda akan menemukan referensi ke Common Weakness Enumeration (CWE). Mari kita jelaskan alasan untuk merujuk ke daftar ini dan mengapa ini penting dari sudut pandang keamanan.

Seringkali penyebab kerentanan dalam program bukanlah beberapa keadaan rumit, tetapi kesalahan perangkat lunak dangkal. Di sini akan tepat mengutip kutipan ini dari prqa.com :

Institut Nasional Standar dan Teknologi (NIST) melaporkan bahwa 64% kerentanan perangkat lunak berasal dari kesalahan pemrograman dan bukan kurangnya fitur keamanan.

Anda dapat membaca di artikel “ Bagaimana PVS-Studio dapat membantu menemukan kerentanan? ” Dengan beberapa contoh kesalahan sederhana yang menyebabkan kerentanan dalam proyek-proyek seperti MySQL, iOS, NAS, illumos-gate.

Oleh karena itu, banyak kerentanan dapat dihilangkan dengan mendeteksi kesalahan umum dalam waktu dan memperbaikinya. Dan di sini Enumerasi Kelemahan Umum memasuki lokasi.

Kesalahan berbeda, dan tidak semua kesalahan berbahaya dari sudut pandang keamanan. Kesalahan yang berpotensi menyebabkan kerentanan dikumpulkan dalam Penghitungan Kelemahan Umum. Daftar ini sedang diperbarui, dan mungkin ada kesalahan yang dapat menyebabkan kerentanan, tetapi mereka belum dimasukkan dalam daftar ini.

Namun, jika kesalahan diklasifikasikan menurut CWE, maka itu berarti bahwa secara teori dimungkinkan bahwa itu dapat dieksploitasi sebagai kerentanan ( CVE ). Ya, kemungkinan ini kecil. Sangat jarang, CWE berubah menjadi CVE. Namun, jika Anda ingin melindungi kode Anda dari kerentanan, Anda harus, jika mungkin, menemukan sebanyak mungkin bug yang dijelaskan dalam CWE dan memperbaikinya.

Secara skematis, hubungan antara PVS-Studio, kesalahan, CWE dan CVE ditunjukkan pada gambar:

Beberapa kesalahan diklasifikasikan sebagai CWE. PVS-Studio dapat mendeteksi banyak kesalahan ini, sehingga mencegah beberapa cacat ini menjadi kerentanan (CVE).

Kita dapat mengatakan bahwa PVS-Studio mengidentifikasi banyak kerentanan potensial sebelum menyebabkan kerusakan. Dengan demikian, PVS-Studio adalah alat pengujian keamanan aplikasi statis ( SAST ).

Sekarang, saya pikir, jelas mengapa, ketika menggambarkan kesalahan, saya menganggap penting untuk mencatat bagaimana mereka diklasifikasikan menurut CWE. Dengan cara ini lebih mudah untuk menunjukkan pentingnya menggunakan penganalisa kode statis dalam proyek-proyek penting, yang berhubungan dengan sistem operasi.

Cek Android

Untuk menganalisis proyek, saya menggunakan penganalisa PVS-Studio versi 6.24. Penganalisa saat ini mendukung bahasa dan kompiler berikut:

• Windows Visual Studio 2010-2017 C, C ++, C ++ / CLI, C ++ / CX (WinRT), C #
• Windows IAR Embedded Workbench, C / C ++ Compiler untuk ARM C, C ++
• Windows / Linux Keil μVision, DS-MDK, ARM Compiler 5/6 C, C ++
• Windows / Linux Studio Komposer Kode Texas Instruments, Alat Pembuatan Kode ARM C, C ++
• Windows / Linux / macOS. Dentang C, C ++
• Linux / macOS. GCC C, C ++
• Windows MinGW C, C ++

Catatan Mungkin beberapa pembaca kami telah ketinggalan berita bahwa kami mendukung pekerjaan di lingkungan macOS dan mereka akan tertarik dengan publikasi ini: " rilis PVS-Studio untuk macOS: 64 kelemahan di Apple XNU Kernel ".

Proses memeriksa kode sumber Android tidak menjadi masalah, jadi saya tidak akan membahasnya. Masalahnya, alih-alih, adalah keasyikan saya dengan tugas-tugas lain, itulah sebabnya saya tidak menemukan waktu dan energi untuk melihat laporan dengan hati-hati seperti yang saya inginkan. Namun, bahkan melihat sekilas lebih dari cukup untuk mengumpulkan banyak koleksi kesalahan menarik untuk artikel yang solid.

Yang paling penting: Saya meminta pengembang Android tidak hanya untuk memperbaiki kesalahan yang dijelaskan dalam artikel, tetapi juga untuk melakukan analisis independen yang lebih hati-hati. Saya melihat laporan analisa secara dangkal dan bisa melewatkan banyak kesalahan serius.

Pada tes pertama, penganalisa akan menghasilkan banyak positif palsu, tetapi ini bukan masalah . Tim kami siap membantu dengan rekomendasi untuk mengonfigurasi analisator untuk mengurangi jumlah positif palsu. Kami juga siap memberikan kunci lisensi selama sebulan atau lebih, jika perlu. Secara umum, kirim surat kepada kami , kami akan membantu dan memberi tahu Anda.

Sekarang mari kita lihat kesalahan apa dan potensi kerentanan yang berhasil saya temukan. Saya harap Anda menikmati apa yang dapat ditemukan oleh penganalisa kode statis PVS-Studio. Selamat membaca.

Perbandingan tidak berarti

Penganalisa menganggap ekspresi tidak normal jika selalu benar atau salah. Peringatan tersebut, menurut Penghitungan Kelemahan Umum, diklasifikasikan sebagai:

• CWE-570 : Ekspresi Selalu Salah
• CWE-571 : Ekspresi Selalu Benar

Penganalisis menghasilkan banyak peringatan seperti itu, dan, sayangnya, sebagian besar dari mereka salah untuk kode Android. Dalam hal ini, penganalisa tidak bisa disalahkan. Hanya kodenya yang ditulis seperti itu.
Saya akan menunjukkan ini dengan contoh sederhana.

 #if GENERIC_TARGET const char alternative_config_path[] = "/data/nfc/"; #else const char alternative_config_path[] = ""; #endif CNxpNfcConfig& CNxpNfcConfig::GetInstance() { .... if (alternative_config_path[0] != '\0') { .... } 

Di sini penganalisis menghasilkan peringatan: V547 CWE-570 Ekspresi 'alternative_config_path [0]! =' \ 0 '' selalu salah. phNxpConfig.cpp 401

Faktanya adalah bahwa makro GENERIC_TARGET tidak didefinisikan, dan dari sudut pandang penganalisa, kode terlihat seperti ini:

 const char alternative_config_path[] = ""; .... if (alternative_config_path[0] != '\0') { 

Penganalisa hanya berkewajiban untuk mengeluarkan peringatan, karena jalurnya kosong, dan terminal nol selalu berada di nol offset. Dengan demikian, penganalisa secara formal benar dalam mengeluarkan peringatan. Namun, dari sudut pandang praktis, tidak ada manfaat dari peringatan ini.

Sayangnya, tidak ada yang bisa dilakukan dengan situasi seperti itu. Kami harus meninjau secara sistematis semua peringatan semacam itu dan menandai banyak tempat sebagai positif palsu sehingga penganalisa tidak lagi mengeluarkan pesan pada baris ini. Ini benar-benar perlu dilakukan, karena, selain pesan yang tidak berarti, banyak kesalahan nyata akan ditemukan.

Jujur saya akui bahwa saya tidak tertarik hati-hati melihat peringatan jenis ini, dan saya pergi mereka secara dangkal. Namun, bahkan ini akan cukup untuk menunjukkan bahwa diagnostik seperti itu sangat berguna dan menemukan kesalahan yang menarik.

Saya ingin memulai dengan situasi klasik ketika fungsi membandingkan dua objek salah diimplementasikan. Mengapa klasik? Ini adalah pola kesalahan khas yang terus-menerus kita temui dalam berbagai proyek. Kemungkinan besar, ada tiga alasan terjadinya:

1. Fungsi perbandingannya sederhana dan ditulis "secara otomatis" dan menggunakan teknologi Copy-Paste. Saat menulis kode seperti itu, seseorang lalai dan sering membuat kesalahan ketik.
2. Biasanya, peninjauan kode tidak dilakukan untuk fungsi-fungsi seperti itu, karena terlalu malas untuk melihat fungsi-fungsi yang sederhana dan membosankan.
3. Tes unit biasanya tidak dilakukan untuk fungsi tersebut. Karena malas. Selain itu, fungsinya sederhana, dan pemrogram tidak berpikir bahwa ada kemungkinan kesalahan di dalamnya.

Pikiran-pikiran dan contoh-contoh ini dijelaskan secara lebih rinci dalam artikel " Jahat hidup dalam fungsi perbandingan ."

 static inline bool isAudioPlaybackRateEqual( const AudioPlaybackRate &pr1, const AudioPlaybackRate &pr2) { return fabs(pr1.mSpeed - pr2.mSpeed) < AUDIO_TIMESTRETCH_SPEED_MIN_DELTA && fabs(pr1.mPitch - pr2.mPitch) < AUDIO_TIMESTRETCH_PITCH_MIN_DELTA && pr2.mStretchMode == pr2.mStretchMode && pr2.mFallbackMode == pr2.mFallbackMode; } 

Jadi, sebelum kita adalah fungsi klasik membandingkan dua objek bertipe AudioPlaybackRate . Dan, seperti yang saya pikirkan, pembaca menduga itu salah. Analyzer PVS-Studio segera mengetahui dua kesalahan ketik:

• V501 CWE-571 Ada sub-ekspresi yang identik ke kiri dan ke kanan operator '==': pr2.mStretchMode == pr2.mStretchMode AudioResamplerPublic.h 107
• V501 CWE-571 Ada sub-ekspresi identik ke kiri dan ke kanan operator '==': pr2.mFallbackMode == pr2.mFallbackMode AudioResamplerPublic.h 108

Bidang pr2.mStretchMode dan bidang pr2.mFallbackMode dibandingkan dengan mereka sendiri. Ternyata fungsinya tidak membandingkan objek dengan cukup akurat.

Perbandingan tidak berguna berikut ini hidup dalam fungsi yang menyimpan informasi sidik jari dalam file.

 static void saveFingerprint(worker_thread_t* listener, int idx) { .... int ns = fwrite(&listener->secureid[idx], sizeof(uint64_t), 1, fp); .... int nf = fwrite(&listener->fingerid[idx], sizeof(uint64_t), 1, fp); if (ns != 1 || ns !=1) // <= ALOGW("Corrupt emulator fingerprints storage; " "could not save fingerprints"); fclose(fp); return; } 

Anomali terdeteksi dalam kode ini oleh dua diagnostik sekaligus:

• V501 CWE-570 Ada sub-ekspresi yang identik ke kiri dan ke kanan '||' operator: ns! = 1 || ns! = 1 sidik jari.c 126
• V560 CWE-570 Bagian dari ekspresi kondisional selalu salah: ns! = 1. fingerprint.c 126

Tidak ada penanganan situasi ketika panggilan kedua ke fungsi fwrite tidak dapat menulis data ke file. Dengan kata lain, nilai variabel nf tidak dicentang. Cek yang benar akan terlihat seperti ini:

 if (ns != 1 || nf != 1) 

Kami melanjutkan ke kesalahan berikutnya yang terkait dengan penggunaan & operator.

 #define O_RDONLY 00000000 #define O_WRONLY 00000001 #define O_RDWR 00000002 static ssize_t verity_read(fec_handle *f, ....) { .... /* if we are in read-only mode and expect to read a zero block, skip reading and just return zeros */ if (f->mode & O_RDONLY && expect_zeros) { memset(data, 0, FEC_BLOCKSIZE); goto valid; } .... } 

Peringatan PVS-Studio: V560 CWE-570 Bagian dari ekspresi kondisional selalu salah: mode f-> & 00000000. fec_read.cpp 322

Perhatikan bahwa konstanta O_RDONLY adalah nol. Ini membuat ekspresi f-> mode & O_RDONLY tidak berguna, karena selalu 0. Ternyata kondisi pernyataan if tidak pernah puas, dan statement-true adalah kode mati.

Cek yang benar harus seperti ini:

 if (f->mode == O_RDONLY && expect_zeros) { 

Sekarang mari kita lihat salah ketik klasik ketika kita lupa untuk menulis bagian dari kondisinya.

 enum { .... CHANGE_DISPLAY_INFO = 1 << 2, .... }; void RotaryEncoderInputMapper::configure(nsecs_t when, const InputReaderConfiguration* config, uint32_t changes) { .... if (!changes || (InputReaderConfiguration::CHANGE_DISPLAY_INFO)) { .... } 

Peringatan PVS-Studio: V768 CWE-571 Konstanta penghitungan 'CHANGE_DISPLAY_INFO' digunakan sebagai variabel tipe-Boolean. InputReader.cpp 3016

Syaratnya selalu benar, karena operan InputReaderConfiguration :: CHANGE_DISPLAY_INFO adalah konstanta sama dengan 4.

Jika Anda melihat bagaimana kode ditulis di lingkungan, menjadi jelas bahwa sebenarnya kondisinya harus seperti ini:

 if (!changes || (changes & InputReaderConfiguration::CHANGE_DISPLAY_INFO)) { 

Perbandingan berikut, yang tidak masuk akal, saya temui di loop operator.

 void parse_printerAttributes(....) { .... ipp_t *collection = ippGetCollection(attrptr, i); for (j = 0, attrptr = ippFirstAttribute(collection); (j < 4) && (attrptr != NULL); attrptr = ippNextAttribute(collection)) { if (strcmp("....", ippGetName(attrptr)) == 0) { ....TopMargin = ippGetInteger(attrptr, 0); } else if (strcmp("....", ippGetName(attrptr)) == 0) { ....BottomMargin = ippGetInteger(attrptr, 0); } else if (strcmp("....", ippGetName(attrptr)) == 0) { ....LeftMargin = ippGetInteger(attrptr, 0); } else if (strcmp("....", ippGetName(attrptr)) == 0) { ....RightMargin = ippGetInteger(attrptr, 0); } } .... } 

Peringatan PVS-Studio: V560 CWE-571 Bagian dari ekspresi kondisional selalu benar: (j <4). ipphelper.c 926

Perhatikan bahwa nilai variabel j tidak bertambah di mana pun. Ini berarti bahwa subekspresi (j <4) selalu benar.

Jumlah terbesar dari operasi yang berguna dari analisa PVS-Studio, mengenai selalu kondisi benar / salah, mengacu pada kode di mana hasil dari membuat objek diperiksa menggunakan operator baru . Dengan kata lain, alat analisa mendeteksi pola kode berikut:

 T *p = new T; if (p == nullptr) return ERROR; 

Pemeriksaan semacam itu tidak ada gunanya. Jika tidak memungkinkan untuk mengalokasikan memori untuk objek, pengecualian dari tipe std :: bad_alloc dilemparkan, dan itu hanya tidak akan sampai ke pengecekan nilai pointer.

Catatan Operator baru dapat mengembalikan nullptr dengan menulis baru (std :: nothrow) T. Namun, ini tidak berlaku untuk kesalahan yang dibahas. Alat analisis PVS-Studio memperhitungkan (std :: nothrow) dan tidak memberikan peringatan jika objek dibuat dengan cara ini.

Tampaknya kesalahan semacam itu tidak berbahaya. Nah, pikirkan itu, pemeriksaan ekstra yang tidak pernah berhasil. Pokoknya, akan ada pengecualian yang akan diproses di suatu tempat. Sayangnya, beberapa pengembang menempatkan pernyataan-benar dari tindakan pernyataan if yang membebaskan sumber daya, dll. Karena kode ini tidak dieksekusi, maka dapat menyebabkan kebocoran memori dan kesalahan lainnya.

Pertimbangkan salah satu dari kasus ini yang saya perhatikan dalam kode Android.

 int parse_apk(const char *path, const char *target_package_name) { .... FileMap *dataMap = zip->createEntryFileMap(entry); if (dataMap == NULL) { ALOGW("%s: failed to create FileMap\n", __FUNCTION__); return -1; } char *buf = new char[uncompLen]; if (NULL == buf) { ALOGW("%s: failed to allocate %" PRIu32 " byte\n", __FUNCTION__, uncompLen); delete dataMap; return -1; } .... } 

Peringatan PVS-Studio: V668 CWE-570 Tidak ada gunanya menguji pointer 'buf' terhadap null, karena memori dialokasikan menggunakan operator 'baru'. Pengecualian akan dihasilkan jika terjadi kesalahan alokasi memori. scan.cpp 213

Harap dicatat bahwa jika tidak mungkin mengalokasikan blok memori kedua, programmer mencoba membebaskan blok pertama:

 delete dataMap; 

Kode ini tidak akan pernah mendapatkan kontrol. Ini kode mati. Jika pengecualian terjadi, kebocoran memori akan terjadi.

Menulis kode seperti itu pada dasarnya salah. Pointer pintar diciptakan untuk kasus seperti itu.

Secara total, penganalisa PVS-Studio mendeteksi 176 tempat dalam kode Android di mana pointer diperiksa setelah membuat objek menggunakan yang baru . Saya tidak mengerti betapa berbahayanya tempat-tempat ini, dan, tentu saja, saya tidak akan mengacaukan artikel dengan semua peringatan ini. Mereka yang tertarik dapat melihat peringatan lain di file Android_V668.txt .

Mendereferensi pointer nol

Mendereferensi pointer nol mengarah ke perilaku program yang tidak terdefinisi, sehingga berguna untuk menemukan dan memperbaiki tempat-tempat tersebut. Bergantung pada situasinya, penganalisis PVS-Studio dapat mengklasifikasikan kesalahan ini menurut Penghitungan Kelemahan Umum sebagai berikut:

• CWE-119 : Pembatasan Operasi yang Tidak Tepat dalam Batas-Batas Penyangga Memori
• CWE-476 : NULL Pointer Dereference
• CWE-628 : Function Call dengan Argumen yang Tidak Benar
• CWE-690 : Nilai Pengembalian Tidak Dicentang ke NULL Pointer Dereference

Saya sering menemukan kesalahan dalam kode yang bertanggung jawab untuk menangani situasi yang tidak standar atau salah. Tidak ada yang menguji kode seperti itu, dan kesalahan dapat hidup di dalamnya untuk waktu yang sangat lama. Kasus seperti itu akan dipertimbangkan sekarang.

 bool parseEffect(....) { .... if (xmlProxyLib == nullptr) { ALOGE("effectProxy must contain a <%s>: %s", tag, dump(*xmlProxyLib)); return false; } .... } 

Peringatan PVS-Studio: V522 CWE-476 Dereferencing dari null pointer 'xmlProxyLib' mungkin terjadi. EffectsConfig.cpp 205

Jika pointer xmlProxyLib adalah nullptr , maka programmer akan menampilkan pesan debug, yang perlu dilakukan dereferensi pointer ini. Ups ...

Sekarang pertimbangkan versi kesalahan yang lebih menarik.

 static void soinfo_unload_impl(soinfo* root) { .... soinfo* needed = find_library(si->get_primary_namespace(), library_name, RTLD_NOLOAD, nullptr, nullptr); if (needed != nullptr) { // <= PRINT("warning: couldn't find %s needed by %s on unload.", library_name, si->get_realpath()); return; } else if (local_unload_list.contains(needed)) { return; } else if (needed->is_linked() && // <= needed->get_local_group_root() != root) { external_unload_list.push_back(needed); } else { unload_list.push_front(needed); } .... } 

Peringatan PVS-Studio: V522 CWE-476 Dereferencing dari null pointer 'diperlukan' mungkin terjadi. linker.cpp 1847

Jika pointer diperlukan! = Nullptr , maka peringatan dikeluarkan, yang merupakan perilaku program yang sangat mencurigakan. Akhirnya menjadi jelas bahwa kode berisi kesalahan jika Anda melihat di bawah dan melihat bahwa ketika diperlukan == nullptr , pointer nol akan ditinjau kembali dalam ekspresi yang diperlukan-> is_linked () .

Kemungkinan besar, operator! = Dan == benar-benar bingung di sini. Jika Anda mengganti, kode fungsi itu masuk akal, dan kesalahannya hilang.

Sebagian besar peringatan tentang potensi dereferencing dari pointer nol mengacu pada situasi formulir:

 T *p = (T *) malloc (N); *p = x; 

Fungsi seperti malloc , strdup, dan sebagainya dapat mengembalikan NULL jika memori tidak dapat dialokasikan. Oleh karena itu, Anda tidak dapat menentukan pointer yang mengembalikan fungsi-fungsi ini tanpa terlebih dahulu memeriksa pointer.

Ada banyak kesalahan serupa, jadi saya hanya akan memberikan dua fragmen kode sederhana: yang pertama dengan
malloc dan yang kedua dengan strdup .

 DownmixerBufferProvider::DownmixerBufferProvider(....) { .... effect_param_t * const param = (effect_param_t *) malloc(downmixParamSize); param->psize = sizeof(downmix_params_t); .... } 

Peringatan PVS-Studio: V522 CWE-690 Mungkin ada dereferensi dari 'param' penunjuk null potensial. Periksa baris: 245, 244. BufferProviders.cpp 245

 static char* descriptorClassToDot(const char* str) { .... newStr = strdup(lastSlash); newStr[strlen(lastSlash)-1] = '\0'; .... } 

Peringatan PVS-Studio: V522 CWE-690 Mungkin ada referensi negatif tentang potensi pointer nol 'newStr'. Periksa baris: 203, 202. DexDump.cpp 203

Seseorang mungkin mengatakan bahwa ini adalah kesalahan kecil. Jika tidak ada cukup memori, maka program hanya akan crash ketika mendereferensi pointer nol, dan ini normal. Karena tidak ada memori, tidak ada yang bisa dilakukan untuk mengatasi situasi ini.

Orang seperti itu salah. Pointer harus diperiksa! Saya memeriksa topik ini secara rinci dalam artikel " Mengapa penting untuk memeriksa apa fungsi malloc dikembalikan ". Saya sangat menyarankan Anda membacanya untuk semua orang yang belum membacanya.

Singkatnya, bahayanya adalah bahwa menulis ke memori mungkin tidak selalu terjadi di dekat alamat nol. Dimungkinkan untuk menulis data di suatu tempat yang sangat jauh ke dalam halaman memori yang tidak dilindungi tulis, dan dengan demikian menyebabkan kesalahan yang sulit dipahami, atau secara umum kesalahan ini dapat digunakan sebagai kerentanan. Mari kita lihat apa yang saya maksud dengan contoh fungsi check_size .

 int check_size(radio_metadata_buffer_t **metadata_ptr, const uint32_t size_int) { .... metadata = realloc(metadata, new_size_int * sizeof(uint32_t)); memmove( (uint32_t *)metadata + new_size_int - (metadata->count + 1), (uint32_t *)metadata + metadata->size_int - (metadata->count + 1), (metadata->count + 1) * sizeof(uint32_t)); .... } 

Peringatan PVS-Studio: V769 CWE-119 Penunjuk '(uint32_t *) metadata' dalam ekspresi '(uint32_t *) metadata + new_size_int' bisa berupa nullptr. Dalam kasus seperti itu, nilai yang dihasilkan akan menjadi tidak masuk akal dan tidak boleh digunakan. Periksa baris: 91, 89. radio_metadata.c 91

Saya tidak mengerti logika fungsi, tetapi ini tidak perlu. Yang utama adalah bahwa buffer baru dibuat dan data disalin di sana. Jika fungsi realloc mengembalikan NULL , maka data akan disalin ke alamat ((uint32_t *) NULL + metadata-> size_int - (metadata-> count + 1)).

Jika nilai metadata-> size_int besar, maka konsekuensinya akan disesalkan. Ternyata data ditulis ke memori acak.

Ngomong-ngomong, ada jenis lain dari null pointer dereferencing, yang digolongkan oleh analyzer PVS-Studio bukan sebagai CWE-690, tetapi sebagai CWE-628 (argumen tidak valid).

 static void parse_tcp_ports(const char *portstring, uint16_t *ports) { char *buffer; char *cp; buffer = strdup(portstring); if ((cp = strchr(buffer, ':')) == NULL) .... } 

Peringatan PVS-Studio: V575 CWE-628 Potensi null pointer dilewatkan ke fungsi 'strchr'. Periksa argumen pertama. Periksa baris: 47, 46. libxt_tcp.c 47

Faktanya adalah bahwa pointer dereferencing akan terjadi ketika fungsi strchr dipanggil . Oleh karena itu, penganalisa mengartikan situasi ini sebagai memberikan nilai yang salah ke fungsi.

Sisa 194 peringatan dari daftar tipe I ini dalam file Android_V522_V575.txt .

Omong-omong, perhatian khusus untuk semua kesalahan ini diberikan oleh peringatan yang dibahas sebelumnya tentang memeriksa pointer setelah memanggil operator baru . Ternyata ada 195 panggilan ke fungsi malloc / realloc / strdup, dan seterusnya, ketika pointer tidak dicentang. Tetapi ada 176 tempat di mana pointer diperiksa setelah memanggil baru . Setuju, pendekatan aneh!

Pada akhirnya, kita hanya perlu mempertimbangkan peringatan V595 dan V1004, yang juga terkait dengan penggunaan pointer nol.

V595 mendeteksi situasi di mana pointer dereferenced dan kemudian diperiksa. Contoh sintetis:

 p->foo(); if (!p) Error(); 

V1004 mengungkapkan situasi terbalik ketika pointer diperiksa terlebih dahulu, dan kemudian lupa melakukannya. Contoh sintetis:

 if (p) p->foo(); p->doo(); 

Mari kita lihat beberapa fragmen kode Android, di mana ada kesalahan jenis ini. .

 PV_STATUS RC_UpdateBuffer(VideoEncData *video, Int currLayer, Int num_skip) { rateControl *rc = video->rc[currLayer]; MultiPass *pMP = video->pMP[currLayer]; if (video == NULL || rc == NULL || pMP == NULL) return PV_FAIL; .... } 

PVS-Studio: V595 CWE-476 The 'video' pointer was utilized before it was verified against nullptr. Check lines: 385, 388. rate_control.cpp 385

 static void resampler_reset(struct resampler_itfe *resampler) { struct resampler *rsmp = (struct resampler *)resampler; rsmp->frames_in = 0; rsmp->frames_rq = 0; if (rsmp != NULL && rsmp->speex_resampler != NULL) { speex_resampler_reset_mem(rsmp->speex_resampler); } } 

PVS-Studio: V595 CWE-476 The 'rsmp' pointer was utilized before it was verified against nullptr. Check lines: 54, 57. resampler.c 54

 void bta_gattc_disc_cmpl(tBTA_GATTC_CLCB* p_clcb, UNUSED_ATTR tBTA_GATTC_DATA* p_data) { .... if (p_clcb->status != GATT_SUCCESS) { if (p_clcb->p_srcb) { std::vector<tBTA_GATTC_SERVICE>().swap( p_clcb->p_srcb->srvc_cache); } bta_gattc_cache_reset(p_clcb->p_srcb->server_bda); } .... } 

PVS-Studio: V1004 CWE-476 The 'p_clcb->p_srcb' pointer was used unsafely after it was verified against nullptr. Check lines: 695, 701. bta_gattc_act.cc 701

. , , - .

:

• V1004 CWE-476 The 'ain' pointer was used unsafely after it was verified against nullptr. Check lines: 101, 105. rsCpuIntrinsicBLAS.cpp 105
• V595 CWE-476 The 'outError' pointer was utilized before it was verified against nullptr. Check lines: 437, 450. Command.cpp 437
• V595 CWE-476 The 'out_last_reference' pointer was utilized before it was verified against nullptr. Check lines: 432, 436. AssetManager2.cpp 432
• V595 CWE-476 The 'set' pointer was utilized before it was verified against nullptr. Check lines: 4524, 4529. ResourceTypes.cpp 4524
• V595 CWE-476 The 'reply' pointer was utilized before it was verified against nullptr. Check lines: 126, 133. Binder.cpp 126
• V595 CWE-476 The 'video' pointer was utilized before it was verified against nullptr. Check lines: 532, 540. rate_control.cpp 532
• V595 CWE-476 The 'video' pointer was utilized before it was verified against nullptr. Check lines: 702, 711. rate_control.cpp 702
• V595 CWE-476 The 'pInfo' pointer was utilized before it was verified against nullptr. Check lines: 251, 254. ResolveInfo.cpp 251
• V595 CWE-476 The 'address' pointer was utilized before it was verified against nullptr. Check lines: 53, 55. DeviceHalHidl.cpp 53
• V595 CWE-476 The 'halAddress' pointer was utilized before it was verified against nullptr. Check lines: 55, 82. DeviceHalHidl.cpp 55

, . , . , .

:

 NJ_EXTERN NJ_INT16 njx_search_word(NJ_CLASS *iwnn, ....) { .... NJ_PREVIOUS_SELECTION_INFO *prev_info = &(iwnn->previous_selection); if (iwnn == NULL) { return NJ_SET_ERR_VAL(NJ_FUNC_NJ_SEARCH_WORD, NJ_ERR_PARAM_ENV_NULL); } .... } 

PVS-Studio: V595 CWE-476 The 'iwnn' pointer was utilized before it was verified against nullptr. Check lines: 686, 689. ndapi.c 686

, , « ». . , iwnn , . , , .

, . , . , , :

1. , : iwnn->previous_selection
2. , undefined behavior
3. , iwnn
4. : if (iwnn == NULL)
5. , ,

" ".

, Common Weakness Enumeration CWE-14 : Compiler Removal of Code to Clear Buffers.

, : memset , .

, , , . , . , :

• . .
• Common Weakness Enumeration. CWE-14 .
• V597 .

, . Android? Tentu saja ada. : proof :).

Android FwdLockGlue_InitializeRoundKeys , .

 static void FwdLockGlue_InitializeRoundKeys() { unsigned char keyEncryptionKey[KEY_SIZE]; .... memset(keyEncryptionKey, 0, KEY_SIZE); // Zero out key data. } 

PVS-Studio: V597 CWE-14 The compiler could delete the 'memset' function call, which is used to flush 'keyEncryptionKey' buffer. The memset_s() function should be used to erase the private data. FwdLockGlue.c 102

keyEncryptionKey . , . , , " — ? ".

memset . «Zero out key data» , .

, release- memset . memset , memset .

10 Android_V597.txt .

, , memset .

 void SHA1Transform(uint32_t state[5], const uint8_t buffer[64]) { uint32_t a, b, c, d, e; .... /* Wipe variables */ a = b = c = d = e = 0; } 

PVS-Studio: V1001 CWE-563 The 'a' variable is assigned but is not used until the end of the function. sha1.c 213

PVS-Studio , , , . CWE-563 : Assignment to Variable without Use. , , , , , CWE-14. , C C++ . a , b , c , d e , .

Unspecified/implementation-defined behavior

, , .

 typedef int32_t GGLfixed; GGLfixed gglFastDivx(GGLfixed n, GGLfixed d) { if ((d>>24) && ((d>>24)+1)) { n >>= 8; d >>= 8; } return gglMulx(n, gglRecip(d)); } 

PVS-Studio: V793 It is odd that the result of the '(d >> 24) + 1' statement is a part of the condition. Perhaps, this statement should have been compared with something else. fixed.cpp 75

, 8 d , . , , , 0x00 0xFF.

. , , (d>>24). , : ((d>>24)+1). . , . Yaitu d 0b11111111'00000000'00000000'00000000, 0b11111111'11111111'11111111'11111111. 1 0xFFFFFFFF int , 0. : -1+1=0. , ((d>>24)+1) , 1. , , , :).

, .

«». : «The value of E1 >> E2 is E1 right-shifted E2 bit positions. If E1 has an unsigned type or if E1 has a signed type and a non-negative value, the value of the result is the integral part of the quotient of E1/2^E2. If E1 has a signed type and a negative value, the resulting value is implementation-defined».

. , , (implementation-defined behavior). , . , ((d>>24)+1) 0, .. .

: . , , , :

 GGLfixed gglFastDivx(GGLfixed n, GGLfixed d) { uint32_t hibits = static_cast<uint32_t>(d) >> 24; if (hibits != 0x00 && hibits != 0xFF) { n >>= 8; d >>= 8; } return gglMulx(n, gglRecip(d)); } 

, , , , , .

. , : .

: « printf ?»

 int i = 5; printf("%d,%d", i++, i++) 

: . . , , Visual C++, «6,5», , :).

, . , , , Android.

 bool ComposerClient::CommandReader::parseSetLayerCursorPosition( uint16_t length) { if (length != CommandWriterBase::kSetLayerCursorPositionLength) { return false; } auto err = mHal.setLayerCursorPosition(mDisplay, mLayer, readSigned(), readSigned()); if (err != Error::NONE) { mWriter.setError(getCommandLoc(), err); } return true; } 

PVS-Studio: V681 CWE-758 The language standard does not define an order in which the 'readSigned' functions will be called during evaluation of arguments. ComposerClient.cpp 836

:

 mHal.setLayerCursorPosition(...., readSigned(), readSigned()); 

readSigned . , . Unspecified Behavior.

PVS-Studio . . , . .

 const std::map<std::string, int32_t> kBootReasonMap = { .... {"watchdog_sdi_apps_reset", 106}, {"smpl", 107}, {"oem_modem_failed_to_powerup", 108}, {"reboot_normal", 109}, {"oem_lpass_cfg", 110}, // <= {"oem_xpu_ns_error", 111}, // <= {"power_key_press", 112}, {"hardware_reset", 113}, {"reboot_by_powerkey", 114}, {"reboot_verity", 115}, {"oem_rpm_undef_error", 116}, {"oem_crash_on_the_lk", 117}, {"oem_rpm_reset", 118}, {"oem_lpass_cfg", 119}, // <= {"oem_xpu_ns_error", 120}, // <= {"factory_cable", 121}, {"oem_ar6320_failed_to_powerup", 122}, {"watchdog_rpm_bite", 123}, {"power_on_cable", 124}, {"reboot_unknown", 125}, .... }; 

PVS-Studio:

• V766 CWE-462 An item with the same key '«oem_lpass_cfg»' has already been added. bootstat.cpp 264
• V766 CWE-462 An item with the same key '«oem_xpu_ns_error»' has already been added. bootstat.cpp 265

( std::map ) . Common Weakness Enumeration — CWE-462 : Duplicate Key in Associative List.

, , , , .

, , .

 MtpResponseCode MyMtpDatabase::getDevicePropertyValue(....) { .... switch (type) { case MTP_TYPE_INT8: packet.putInt8(longValue); break; case MTP_TYPE_UINT8: packet.putUInt8(longValue); break; case MTP_TYPE_INT16: packet.putInt16(longValue); break; case MTP_TYPE_UINT16: packet.putUInt16(longValue); break; case MTP_TYPE_INT32: packet.putInt32(longValue); break; case MTP_TYPE_UINT32: packet.putUInt32(longValue); break; case MTP_TYPE_INT64: packet.putInt64(longValue); break; case MTP_TYPE_UINT64: packet.putUInt64(longValue); break; case MTP_TYPE_INT128: packet.putInt128(longValue); break; case MTP_TYPE_UINT128: packet.putInt128(longValue); // <= break; .... } 

PVS-Studio: V525 CWE-682 The code contains the collection of similar blocks. Check items 'putInt8', 'putUInt8', 'putInt16', 'putUInt16', 'putInt32', 'putUInt32', 'putInt64', 'putUInt64', 'putInt128', 'putInt128' in lines 620, 623, 626, 629, 632, 635, 638, 641, 644, 647. android_mtp_MtpDatabase.cpp 620

MTP_TYPE_UINT128 putUInt128 , putInt128 .

Copy-Paste.

 static void btif_rc_upstreams_evt(....) { .... case AVRC_PDU_REQUEST_CONTINUATION_RSP: { BTIF_TRACE_EVENT( "%s() REQUEST CONTINUATION: target_pdu: 0x%02d", __func__, pavrc_cmd->continu.target_pdu); tAVRC_RESPONSE avrc_rsp; if (p_dev->rc_connected == TRUE) { memset(&(avrc_rsp.continu), 0, sizeof(tAVRC_NEXT_RSP)); avrc_rsp.continu.opcode = opcode_from_pdu(AVRC_PDU_REQUEST_CONTINUATION_RSP); avrc_rsp.continu.pdu = AVRC_PDU_REQUEST_CONTINUATION_RSP; avrc_rsp.continu.status = AVRC_STS_NO_ERROR; avrc_rsp.continu.target_pdu = pavrc_cmd->continu.target_pdu; send_metamsg_rsp(p_dev, -1, label, ctype, &avrc_rsp); } } break; case AVRC_PDU_ABORT_CONTINUATION_RSP: { BTIF_TRACE_EVENT( "%s() ABORT CONTINUATION: target_pdu: 0x%02d", __func__, pavrc_cmd->abort.target_pdu); tAVRC_RESPONSE avrc_rsp; if (p_dev->rc_connected == TRUE) { memset(&(avrc_rsp.abort), 0, sizeof(tAVRC_NEXT_RSP)); avrc_rsp.abort.opcode = opcode_from_pdu(AVRC_PDU_ABORT_CONTINUATION_RSP); avrc_rsp.abort.pdu = AVRC_PDU_ABORT_CONTINUATION_RSP; avrc_rsp.abort.status = AVRC_STS_NO_ERROR; avrc_rsp.abort.target_pdu = pavrc_cmd->continu.target_pdu; send_metamsg_rsp(p_dev, -1, label, ctype, &avrc_rsp); } } break; .... } 

, .

, . , Java, .

, , . : V778 CWE-682 Two similar code fragments were found. Perhaps, this is a typo and 'abort' variable should be used instead of 'continu'. btif_rc.cc 1554

, Copy-Paste, , , . " continu " " abort ".

Yaitu :

 avrc_rsp.abort.target_pdu = pavrc_cmd->abort.target_pdu; 

" ", .

Facepalm

little-endian big-endian (. ).

 inline uint32_t bswap32(uint32_t pData) { return (((pData & 0xFF000000) >> 24) | ((pData & 0x00FF0000) >> 8) | ((pData & 0x0000FF00) << 8) | ((pData & 0x000000FF) << 24)); } bool ELFAttribute::merge(....) { .... uint32_t subsection_length = *reinterpret_cast<const uint32_t*>(subsection_data); if (llvm::sys::IsLittleEndianHost != m_Config.targets().isLittleEndian()) bswap32(subsection_length); .... } 

PVS-Studio: V530 CWE-252 The return value of function 'bswap32' is required to be utilized. ELFAttribute.cpp 84

bswap32 . :

 bswap32(subsection_length); 

, . . .

CWE-252 : Unchecked Return Value. , , CWE-198 : Use of Incorrect Byte Ordering. , . , .

:

 subsection_length = bswap32(subsection_length); 

Android :

• V530 CWE-252 The return value of function 'bswap32' is required to be utilized. ELFAttribute.cpp 218
• V530 CWE-252 The return value of function 'bswap32' is required to be utilized. ELFAttribute.cpp 346
• V530 CWE-252 The return value of function 'bswap32' is required to be utilized. ELFAttribute.cpp 352

, [[nodiscard]] . , , . , :

 [[nodiscard]] inline uint32_t bswap32(uint32_t pData) { ... } 

. " ++17 ".

, , .

Common Weakness Enumeration — CWE-561 : Dead Code.

 virtual sp<IEffect> createEffect(....) { .... if (pDesc == NULL) { return effect; if (status != NULL) { *status = BAD_VALUE; } } .... } 

PVS-Studio: V779 CWE-561 Unreachable code detected. It is possible that an error is present. IAudioFlinger.cpp 733

return .

:

• V779 CWE-561 Unreachable code detected. It is possible that an error is present. bta_hf_client_main.cc 612
• V779 CWE-561 Unreachable code detected. It is possible that an error is present. android_media_ImageReader.cpp 468
• V779 CWE-561 Unreachable code detected. It is possible that an error is present. AMPEG4AudioAssembler.cpp 187

break

break switch — C C++ . , C++17 , [[fallthrough]] . [[fallthrough]] " break fallthrough ".

, [[fallthrough]] , PVS-Studio. , Android. Common Weakness Enumeration, CWE-484 : Omitted Break Statement in Switch.

 bool A2dpCodecConfigLdac::setCodecConfig(....) { .... case BTAV_A2DP_CODEC_SAMPLE_RATE_192000: if (sampleRate & A2DP_LDAC_SAMPLING_FREQ_192000) { result_config_cie.sampleRate = A2DP_LDAC_SAMPLING_FREQ_192000; codec_capability_.sample_rate = codec_user_config_.sample_rate; codec_config_.sample_rate = codec_user_config_.sample_rate; } case BTAV_A2DP_CODEC_SAMPLE_RATE_16000: case BTAV_A2DP_CODEC_SAMPLE_RATE_24000: case BTAV_A2DP_CODEC_SAMPLE_RATE_NONE: codec_capability_.sample_rate = BTAV_A2DP_CODEC_SAMPLE_RATE_NONE; codec_config_.sample_rate = BTAV_A2DP_CODEC_SAMPLE_RATE_NONE; break; .... } 

PVS-Studio: V796 CWE-484 It is possible that 'break' statement is missing in switch statement. a2dp_vendor_ldac.cc 912

, . , . , V519:

• V519 CWE-563 The 'codec_capability_.sample_rate' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 910, 916. a2dp_vendor_ldac.cc 916
• V519 CWE-563 The 'codec_config_.sample_rate' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 911, 917. a2dp_vendor_ldac.cc 917

:

 Return<void> EffectsFactory::getAllDescriptors(....) { .... switch (status) { case -ENOSYS: { // Effect list has changed. goto restart; } case -ENOENT: { // No more effects available. result.resize(i); } default: { result.resize(0); retval = Result::NOT_INITIALIZED; } } .... } 

PVS-Studio: V796 CWE-484 It is possible that 'break' statement is missing in switch statement. EffectsFactory.cpp 118

 int Reverb_getParameter(....) { .... case REVERB_PARAM_REFLECTIONS_LEVEL: *(uint16_t *)pValue = 0; case REVERB_PARAM_REFLECTIONS_DELAY: *(uint32_t *)pValue = 0; case REVERB_PARAM_REVERB_DELAY: *(uint32_t *)pValue = 0; break; .... } 

PVS-Studio: V796 CWE-484 It is possible that 'break' statement is missing in switch statement. EffectReverb.cpp 1847

 static SLresult IAndroidConfiguration_GetConfiguration(....) { .... switch (IObjectToObjectID((thiz)->mThis)) { case SL_OBJECTID_AUDIORECORDER: result = android_audioRecorder_getConfig( (CAudioRecorder *) thiz->mThis, configKey, pValueSize, pConfigValue); break; case SL_OBJECTID_AUDIOPLAYER: result = android_audioPlayer_getConfig( (CAudioPlayer *) thiz->mThis, configKey, pValueSize, pConfigValue); default: result = SL_RESULT_FEATURE_UNSUPPORTED; break; } .... } 

PVS-Studio: V796 CWE-484 It is possible that 'break' statement is missing in switch statement. IAndroidConfiguration.cpp 90

, . , Common Weakness Enumeration, :

• CWE-401 : Improper Release of Memory Before Removing Last Reference ('Memory Leak')
• CWE-562 : Return of Stack Variable Address
• CWE-762 : Mismatched Memory Management Routines

, .

 TransformIterator& operator++(int) { TransformIterator tmp(*this); ++*this; return tmp; } TransformIterator& operator--(int) { TransformIterator tmp(*this); --*this; return tmp; } 

PVS-Studio:

• V558 CWE-562 Function returns the reference to temporary local object: tmp. transform_iterator.h 77
• V558 CWE-562 Function returns the reference to temporary local object: tmp. transform_iterator.h 92

, tmp , . , () .

:

 TransformIterator operator++(int) { TransformIterator tmp(*this); ++*this; return tmp; } TransformIterator operator--(int) { TransformIterator tmp(*this); --*this; return tmp; } 

, .

 int register_socket_transport( int s, const char* serial, int port, int local) { atransport* t = new atransport(); if (!serial) { char buf[32]; snprintf(buf, sizeof(buf), "T-%p", t); serial = buf; } .... } 

PVS-Studio: V507 CWE-562 Pointer to local array 'buf' is stored outside the scope of this array. Such a pointer will become invalid. transport.cpp 1030

. serial NULL, . if , buf . , , , . , .

.

 void SensorService::SensorEventConnection::reAllocateCacheLocked(....) { sensors_event_t *eventCache_new; const int new_cache_size = computeMaxCacheSizeLocked(); eventCache_new = new sensors_event_t[new_cache_size]; .... delete mEventCache; mEventCache = eventCache_new; mCacheSize += count; mMaxCacheSize = new_cache_size; } 

PVS-Studio: V611 CWE-762 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] mEventCache;'. Check lines: 391, 384. SensorEventConnection.cpp 391

. , mEventCache , new [] . , delete . .

:

 aaudio_result_t AAudioServiceEndpointCapture::open(....) { .... delete mDistributionBuffer; int distributionBufferSizeBytes = getStreamInternal()->getFramesPerBurst() * getStreamInternal()->getBytesPerFrame(); mDistributionBuffer = new uint8_t[distributionBufferSizeBytes]; .... } 

PVS-Studio: V611 CWE-762 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] mDistributionBuffer;'. AAudioServiceEndpointCapture.cpp 50

, .

, .

 struct HeifFrameInfo { .... void set(....) { .... mIccData.reset(new uint8_t[iccSize]); .... } .... std::unique_ptr<uint8_t> mIccData; }; 

V554 CWE-762 Incorrect use of unique_ptr. Memori yang dialokasikan dengan 'baru [] akan dibersihkan menggunakan' hapus '. HeifDecoderAPI.h 62

std::unique_ptr delete . set new [] .

Opsi yang benar:

 std::unique_ptr<uint8_t[]> mIccData; 

Kesalahan lain:

• V554 CWE-762 Incorrect use of unique_ptr. Memori yang dialokasikan dengan 'baru [] akan dibersihkan menggunakan' hapus '. atrace.cpp 949
• V554 CWE-762 Incorrect use of unique_ptr. Memori yang dialokasikan dengan 'baru [] akan dibersihkan menggunakan' hapus '. atrace.cpp 950
• V554 CWE-762 Incorrect use of unique_ptr. Memori yang dialokasikan dengan 'baru [] akan dibersihkan menggunakan' hapus '. HeifDecoderImpl.cpp 102
• V554 CWE-762 Incorrect use of unique_ptr. Memori yang dialokasikan dengan 'baru [] akan dibersihkan menggunakan' hapus '. HeifDecoderImpl.cpp 166
• V554 CWE-762 Incorrect use of unique_ptr. Memori yang dialokasikan dengan 'baru [] akan dibersihkan menggunakan' hapus '. ColorSpace.cpp 360

, . , 20. , , .

 Asset* Asset::createFromUncompressedMap(FileMap* dataMap, AccessMode mode) { _FileAsset* pAsset; status_t result; pAsset = new _FileAsset; result = pAsset->openChunk(dataMap); if (result != NO_ERROR) return NULL; pAsset->mAccessMode = mode; return pAsset; } 

PVS-Studio: V773 CWE-401 The function was exited without releasing the 'pAsset' pointer. Kebocoran memori dimungkinkan. Asset.cpp 296

chunk, , pAsset . .

, . : Android_V773.txt .

, . Android :

 if (i < 0 || i > MAX) return; A[i] = x; 

C C++ 0, , . :

 if (i < 0 || i >= MAX) return; A[i] = x; 

, Common Weakness Enumeration, CWE-119 : Improper Restriction of Operations within the Bounds of a Memory Buffer.

, Android.

 static btif_hf_cb_t btif_hf_cb[BTA_AG_MAX_NUM_CLIENTS]; static bool IsSlcConnected(RawAddress* bd_addr) { if (!bd_addr) { LOG(WARNING) << __func__ << ": bd_addr is null"; return false; } int idx = btif_hf_idx_by_bdaddr(bd_addr); if (idx < 0 || idx > BTA_AG_MAX_NUM_CLIENTS) { LOG(WARNING) << __func__ << ": invalid index " << idx << " for " << *bd_addr; return false; } return btif_hf_cb[idx].state == BTHF_CONNECTION_STATE_SLC_CONNECTED; } 

PVS-Studio: V557 CWE-119 Array overrun is possible. The value of 'idx' index could reach 6. btif_hf.cc 277

:

 if (idx < 0 || idx >= BTA_AG_MAX_NUM_CLIENTS) { 

:

• V557 CWE-119 Array overrun is possible. The value of 'idx' index could reach 6. btif_hf.cc 869
• V557 CWE-119 Array overrun is possible. The value of 'index' index could reach 6. btif_rc.cc 374

. Android , , Common Weakness Enumeration, :

• CWE-20 : Improper Input Validation
• CWE-670 : Always-Incorrect Control Flow Implementation
• CWE-691 : Insufficient Control Flow Management
• CWE-834 : Excessive Iteration

, , « » , .

 int main(int argc, char **argv) { .... char c; printf("%s is already in *.base_fs format, just ..... ", ....); rewind(blk_alloc_file); while ((c = fgetc(blk_alloc_file)) != EOF) { fputc(c, base_fs_file); } .... } 

PVS-Studio: V739 CWE-20 EOF should not be compared with a value of the 'char' type. The '(c = fgetc(blk_alloc_file))' should be of the 'int' type. blk_alloc_to_base_fs.c 61

, EOF char . , .

fgetc int , : 0 255 EOF (-1). char . - 0xFF (255) -1 , (EOF).

- , Extended ASCII Codes, , . , Windows-1251 0xFF .

, , . , c int.

for .

 status_t AudioPolicyManager::registerPolicyMixes(....) { .... for (size_t i = 0; i < mixes.size(); i++) { .... for (size_t j = 0; i < mHwModules.size(); j++) { // <= if (strcmp(AUDIO_HARDWARE_MODULE_ID_REMOTE_SUBMIX, mHwModules[j]->mName) == 0 && mHwModules[j]->mHandle != 0) { rSubmixModule = mHwModules[j]; break; } .... } .... } 

PVS-Studio: V534 CWE-691 It is likely that a wrong variable is being compared inside the 'for' operator. Consider reviewing 'i'. AudioPolicyManager.cpp 2489

- i , j . j , mHwModules . , , .

, . : AudioPolicyManager.cpp 2586.

3 , . , , . , .

:

 void ce_t3t_handle_check_cmd(....) { .... for (i = 0; i < p_cb->cur_cmd.num_blocks; i++) { .... for (i = 0; i < T3T_MSG_NDEF_ATTR_INFO_SIZE; i++) { checksum += p_temp[i]; } .... } .... } 

PVS-Studio: V535 CWE-691 The variable 'i' is being used for this loop and for the outer loop. Check lines: 398, 452. ce_t3t.cc 452

, i , .

:

• V535 CWE-691 The variable 'xx' is being used for this loop and for the outer loop. Check lines: 801, 807. sdp_db.cc 807
• V535 CWE-691 The variable 'xx' is being used for this loop and for the outer loop. Check lines: 424, 438. nfa_hci_act.cc 438

? PVS-Studio , .

.

 #define NFA_HCI_LAST_PROP_GATE 0xFF tNFA_HCI_DYN_GATE* nfa_hciu_alloc_gate(uint8_t gate_id, tNFA_HANDLE app_handle) { .... for (gate_id = NFA_HCI_FIRST_HOST_SPECIFIC_GENERIC_GATE; gate_id <= NFA_HCI_LAST_PROP_GATE; gate_id++) { if (gate_id == NFA_HCI_CONNECTIVITY_GATE) gate_id++; if (nfa_hciu_find_gate_by_gid(gate_id) == NULL) break; } if (gate_id > NFA_HCI_LAST_PROP_GATE) { LOG(ERROR) << StringPrintf( "nfa_hci_alloc_gate - no free Gate ID: %u " "App Handle: 0x%04x", gate_id, app_handle); return (NULL); } .... } 

PVS-Studio: V654 CWE-834 The condition 'gate_id <= 0xFF' of loop is always true. nfa_hci_utils.cc 248

:

• NFA_HCI_LAST_PROP_GATE 0xFF.
• uint8_t . , [0..0xFF].

, gate_id <= NFA_HCI_LAST_PROP_GATE .

CWE-834, CWE-571: Expression is Always True.

.

 status_t SimpleDecodingSource::doRead(....) { .... for (int retries = 0; ++retries; ) { .... } 

PVS-Studio: V654 CWE-834 The condition '++ retries' of loop is always true. SimpleDecodingSource.cpp 226

, , retries int .

, ++retries 0. , . , . , . , .

.

 status_t Check(const std::string& source) { .... int pass = 1; .... do { .... switch(rc) { case 0: SLOGI("Filesystem check completed OK"); return 0; case 2: SLOGE("Filesystem check failed (not a FAT filesystem)"); errno = ENODATA; return -1; case 4: if (pass++ <= 3) { SLOGW("Filesystem modified - rechecking (pass %d)", pass); continue; // <= } SLOGE("Failing check after too many rechecks"); errno = EIO; return -1; case 8: SLOGE("Filesystem check failed (no filesystem)"); errno = ENODATA; return -1; default: SLOGE("Filesystem check failed (unknown exit code %d)", rc); errno = EIO; return -1; } } while (0); // <= return 0; } 

PVS-Studio: V696 CWE-670 The 'continue' operator will terminate 'do {… } while (FALSE)' loop because the condition is always false. Check lines: 105, 121. Vfat.cpp 105

:

 do { .... if (x) continue; .... } while (0) 

, continue . Ini salah. continue , . , .

, , , :

 for (;;) { .... if (x) continue; .... break; } 

, . - Copy-Paste. Common Weakness Enumeration, CWE-563 : Assignment to Variable without Use. Android.

 status_t XMLNode::flatten_node(....) const { .... memset(&namespaceExt, 0, sizeof(namespaceExt)); if (mNamespacePrefix.size() > 0) { namespaceExt.prefix.index = htodl(strings.offsetForString(mNamespacePrefix)); } else { namespaceExt.prefix.index = htodl((uint32_t)-1); } namespaceExt.prefix.index = htodl(strings.offsetForString(mNamespacePrefix)); namespaceExt.uri.index = htodl(strings.offsetForString(mNamespaceUri)); .... } 

PVS-Studio: V519 CWE-563 The 'namespaceExt.prefix.index' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1535, 1539. XMLNode.cpp 1539

, :

 if (a > 0) X = 1; else X = 2; X = 1; 

, X ( namespaceExt.prefix.index ) .

 bool AudioFlinger::RecordThread::threadLoop() { .... size_t framesToRead = mBufferSize / mFrameSize; framesToRead = min(mRsmpInFramesOA - rear, mRsmpInFramesP2 / 2); .... } 

PVS-Studio: V519 CWE-563 The 'framesToRead' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 6341, 6342. Threads.cpp 6342

, , . - .

 void SchedulingLatencyVisitorARM::VisitArrayGet(....) { .... if (index->IsConstant()) { last_visited_latency_ = kArmMemoryLoadLatency; } else { if (has_intermediate_address) { } else { last_visited_internal_latency_ += kArmIntegerOpLatency; } last_visited_internal_latency_ = kArmMemoryLoadLatency; } .... } 

PVS-Studio: V519 CWE-563 The 'last_visited_internal_latency_' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 680, 682. scheduler_arm.cc 682

, . , :

 last_visited_internal_latency_ += kArmMemoryLoadLatency; 

, , , code review.

 void multiprecision_fast_mod(uint32_t* c, uint32_t* a) { uint32_t U; uint32_t V; .... c[0] += U; V = c[0] < U; c[1] += V; V = c[1] < V; c[2] += V; // V = c[2] < V; // <= c[2] += U; // V = c[2] < U; // <= c[3] += V; V = c[3] < V; c[4] += V; V = c[4] < V; c[5] += V; V = c[5] < V; .... } 

PVS-Studio: V519 CWE-563 The 'V' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 307, 309. p_256_multprecision.cc 309

« », . : , .

, . , .

 void TagMonitor::parseTagsToMonitor(String8 tagNames) { std::lock_guard<std::mutex> lock(mMonitorMutex); // Expand shorthands if (ssize_t idx = tagNames.find("3a") != -1) { ssize_t end = tagNames.find(",", idx); char* start = tagNames.lockBuffer(tagNames.size()); start[idx] = '\0'; .... } .... } 

PVS-Studio: V593 CWE-783 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. TagMonitor.cpp 50

Common Weakness Enumeration: CWE-783 : Operator Precedence Logic Error.

. «3a» idx . (idx != -1), , idx .

, . :

 if (ssize_t idx = (tagNames.find("3a") != -1)) 

, «3a», false/true idx . idx 0 1.

( idx 1), , idx . 1 .

, :

 ssize_t idx = tagNames.find("3a"); if (idx != -1) 

C++17 :

 if (ssize_t idx = tagNames.find("3a"); idx != -1) 

 struct HearingDevice { .... HearingDevice() { HearingDevice(RawAddress::kEmpty, false); } .... }; 

PVS-Studio: V603 CWE-665 The object was created but it is not being used. If you wish to call constructor, 'this->HearingDevice::HearingDevice(....)' should be used. hearing_aid.cc 176

Common Weakness Enumeration: CWE-665 : Improper Initialization.

, . . . , .

. HearingDevice . .

, ( C++11). :

 HearingDevice() : HearingDevice(RawAddress::kEmpty, false) { } 

 int NET_RecvFrom(int s, void *buf, int len, unsigned int flags, struct sockaddr *from, int *fromlen) { socklen_t socklen = *fromlen; BLOCKING_IO_RETURN_INT( s, recvfrom(s, buf, len, flags, from, &socklen) ); *fromlen = socklen; } 

PVS-Studio: V591 CWE-393 Non-void function should return a value. linux_close.cpp 139

Common Weakness Enumeration: CWE-393 : Return of Wrong Status Code.

. : V591 CWE-393 Non-void function should return a value. linux_close.cpp 158

 int MtpFfsHandle::handleControlRequest(....) { .... struct mtp_device_status *st = reinterpret_cast<struct mtp_device_status*>(buf.data()); st->wLength = htole16(sizeof(st)); .... } 

PVS-Studio: V568 It's odd that 'sizeof()' operator evaluates the size of a pointer to a class, but not the size of the 'st' class object. MtpFfsHandle.cpp 251

, - wLength , . :

 st->wLength = htole16(sizeof(*st)); 

:

• V568 It's odd that 'sizeof()' operator evaluates the size of a pointer to a class, but not the size of the 'cacheinfo' class object. NetlinkEvent.cpp 220
• V568 It's odd that 'sizeof()' operator evaluates the size of a pointer to a class, but not the size of the 'page->next' class object. linker_block_allocator.cpp 146
• V568 It's odd that the argument of sizeof() operator is the '& session_id' expression. reference-ril.c 1775

 #define EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR 0x00000001 #define EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR 0x00000002 #define EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR 0x00000004 EGLContext eglCreateContext(....) { .... case EGL_CONTEXT_FLAGS_KHR: if ((attrib_val | EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR) || (attrib_val | EGL_CONTEXT_OPENGL_FORWARD_C....) || (attrib_val | EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR)) { context_flags = attrib_val; } else { RETURN_ERROR(EGL_NO_CONTEXT,EGL_BAD_ATTRIBUTE); } .... } 

PVS-Studio: V617 CWE-480 Consider inspecting the condition. The '0x00000001' argument of the '|' bitwise operation contains a non-zero value. egl.cpp 1329

Common Weakness Enumeration: CWE-480 : Use of Incorrect Operator.

(A | 1) || (A | 2) || (A | 4) , true. , & , :

 if ((attrib_val & EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR) || (attrib_val & EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR) || (attrib_val & EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR)) 

: V617 CWE-480 Consider inspecting the condition. The '0x00000001' argument of the '|' bitwise operation contains a non-zero value. egl.cpp 1338

 template <typename AddressType> struct RegsInfo { .... uint64_t saved_reg_map = 0; AddressType saved_regs[64]; .... inline AddressType* Save(uint32_t reg) { if (reg > sizeof(saved_regs) / sizeof(AddressType)) { abort(); } saved_reg_map |= 1 << reg; saved_regs[reg] = (*regs)[reg]; return &(*regs)[reg]; } .... } 

PVS-Studio: V629 CWE-190 Consider inspecting the '1 << reg' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. RegsInfo.h 47

Common Weakness Enumeration: CWE-190 : Integer Overflow or Wraparound.

1 << reg reg [0..63]. , , 2^0 2^63.

. , 1 32- int . 1^31. .

:

 saved_reg_map |= static_cast<uint64_t>(1) << reg; 

:

 saved_reg_map |= 1ULL << reg; 

 void PCLmGenerator::writeJobTicket() { // Write JobTicket char inputBin[256]; char outputBin[256]; if (!m_pPCLmSSettings) { return; } getInputBinString(m_pPCLmSSettings->userInputBin, &inputBin[0]); getOutputBin(m_pPCLmSSettings->userOutputBin, &outputBin[0]); strcpy(inputBin, inputBin); strcpy(outputBin, outputBin); .... } 

PVS-Studio:

• V549 CWE-688 The first argument of 'strcpy' function is equal to the second argument. genPCLm.cpp 1181
• V549 CWE-688 The first argument of 'strcpy' function is equal to the second argument. genPCLm.cpp 1182

Common Weakness Enumeration: CWE-688 : Function Call With Incorrect Variable or Reference as Argument.

- . , - .

 void mca_set_cfg_by_tbl(....) { tMCA_DCB* p_dcb; const tL2CAP_FCR_OPTS* p_opt; tMCA_FCS_OPT fcs = MCA_FCS_NONE; if (p_tbl->tcid == MCA_CTRL_TCID) { p_opt = &mca_l2c_fcr_opts_def; } else { p_dcb = mca_dcb_by_hdl(p_tbl->cb_idx); if (p_dcb) { p_opt = &p_dcb->p_chnl_cfg->fcr_opt; fcs = p_dcb->p_chnl_cfg->fcs; } } memset(p_cfg, 0, sizeof(tL2CAP_CFG_INFO)); p_cfg->mtu_present = true; p_cfg->mtu = p_tbl->my_mtu; p_cfg->fcr_present = true; memcpy(&p_cfg->fcr, p_opt, sizeof(tL2CAP_FCR_OPTS)); // <= .... } 

PVS-Studio: V614 CWE-824 Potentially uninitialized pointer 'p_opt' used. Consider checking the second actual argument of the 'memcpy' function. mca_main.cc 252

Common Weakness Enumeration: CWE-824 : Access of Uninitialized Pointer.

p_tbl->tcid != MCA_CTRL_TCID p_dcb == nullptr , p_opt .

 struct timespec { __time_t tv_sec; /* Seconds. */ long int tv_nsec; /* Nanoseconds. */ }; static inline timespec NsToTimespec(int64_t ns) { timespec t; int32_t remainder; t.tv_sec = ns / kNanosPerSecond; remainder = ns % kNanosPerSecond; if (remainder < 0) { t.tv_nsec--; remainder += kNanosPerSecond; } t.tv_nsec = remainder; return t; } 

PVS-Studio: V614 CWE-457 Uninitialized variable 't.tv_nsec' used. clock_ns.h 55

Common Weakness Enumeration: CWE-457 : Use of Uninitialized Variable.

t.tv_nsec . : t.tv_nsec = remainder; . - .

 void bta_dm_co_ble_io_req(....) { .... *p_auth_req = bte_appl_cfg.ble_auth_req | (bte_appl_cfg.ble_auth_req & 0x04) | ((*p_auth_req) & 0x04); .... } 

PVS-Studio: V578 An odd bitwise operation detected. Consider verifying it. bta_dm_co.cc 259

. (bte_appl_cfg.ble_auth_req & 0x04) , . , - .

 bool RSReflectionCpp::genEncodedBitCode() { FILE *pfin = fopen(mBitCodeFilePath.c_str(), "rb"); if (pfin == nullptr) { fprintf(stderr, "Error: could not read file %s\n", mBitCodeFilePath.c_str()); return false; } unsigned char buf[16]; int read_length; mOut.indent() << "static const unsigned char __txt[] ="; mOut.startBlock(); while ((read_length = fread(buf, 1, sizeof(buf), pfin)) > 0) { mOut.indent(); for (int i = 0; i < read_length; i++) { char buf2[16]; snprintf(buf2, sizeof(buf2), "0x%02x,", buf[i]); mOut << buf2; } mOut << "\n"; } mOut.endBlock(true); mOut << "\n"; return true; } 

PVS-Studio: V773 CWE-401 The function was exited without releasing the 'pfin' handle. Kebocoran sumber daya mungkin terjadi. slang_rs_reflection_cpp.cpp 448

, Common Weakness Enumeration, : CWE-401 : Improper Release of Memory Before Removing Last Reference ('Memory Leak'). CWE-775 : Missing Release of File Descriptor or Handle after Effective Lifetime. PVS-Studio.

pfin . fclose . , , .

Kesimpulan

, , Android, PVS-Studio . , weakness ( ) :

• CWE-14: Compiler Removal of Code to Clear Buffers
• CWE-20: Improper Input Validation
• CWE-119: Improper Restriction of Operations within the Bounds of a Memory Buffer
• CWE-190: Integer Overflow or Wraparound
• CWE-198: Use of Incorrect Byte Ordering
• CWE-393: Return of Wrong Status Code
• CWE-401: Improper Release of Memory Before Removing Last Reference ('Memory Leak')
• CWE-457: Use of Uninitialized Variable
• CWE-462: Duplicate Key in Associative List
• CWE-480: Use of Incorrect Operator
• CWE-484: Omitted Break Statement in Switch
• CWE-561: Dead Code
• CWE-562: Return of Stack Variable Address
• CWE-563: Assignment to Variable without Use
• CWE-570: Expression is Always False
• CWE-571: Expression is Always True
• CWE-476: NULL Pointer Dereference
• CWE-628: Function Call with Incorrectly Specified Arguments
• CWE-665: Improper Initialization
• CWE-670: Always-Incorrect Control Flow Implementation
• CWE-682: Incorrect Calculation
• CWE-688: Function Call With Incorrect Variable or Reference as Argument
• CWE-690: Unchecked Return Value to NULL Pointer Dereference
• CWE-691: Insufficient Control Flow Management
• CWE-758: Reliance on Undefined, Unspecified, or Implementation-Defined Behavior
• CWE-762: Mismatched Memory Management Routines
• CWE-775: Missing Release of File Descriptor or Handle after Effective Lifetime
• CWE-783: Operator Precedence Logic Error
• CWE-824: Access of Uninitialized Pointer
• CWE-834: Excessive Iteration

490 . , , , , .

2168000 C C++. 14,4% — . , 1855000 .

, 490 CWE 1855000 .

, PVS-Studio 1 weakness ( ) 4000 Android. , .

Terima kasih atas perhatian anda! :

1. PVS-Studio .
2. : : .
3. , : twitter , RSS , vk.com .

Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Andrey Karpov. We Checked the Android Source Codes by PVS-Studio or Nothing is Perfect