Petualangan dengan klien email Mozilla Thunderbird dimulai dengan pembaruan otomatis ke versi 68.0. Lebih banyak teks dalam pemberitahuan pop-up dan tema gelap default adalah fitur penting dari versi ini. Kadang-kadang saya menemukan kesalahan yang saya inginkan untuk dideteksi dengan analisis statis. Ini menjadi alasan untuk memeriksa kode sumber proyek menggunakan PVS-Studio. Kebetulan pada saat analisis, bug sudah diperbaiki. Namun, karena kami telah memperhatikan proyek, tidak ada alasan untuk tidak menulis tentang cacat yang ditemukan lainnya.
Pendahuluan
Tema gelap versi Thunderbird baru terlihat cantik. Saya suka tema gelap. Saya sudah beralih ke mereka di messenger, Windows, macOS. Segera iPhone akan diperbarui ke iOS 13 dengan tema gelap. Untuk alasan ini saya bahkan harus mengubah iPhone 5S saya untuk model yang lebih baru. Dalam praktiknya, ternyata tema gelap membutuhkan lebih banyak upaya bagi pengembang untuk mengambil warna antarmuka. Tidak semua orang bisa menanganinya pertama kali.
Ini adalah bagaimana tag standar terlihat di Thunderbird setelah memperbarui:
Saya biasanya menggunakan 6 tag (5 standar +1 kustom) untuk menandai email. Setengah dari mereka menjadi tidak mungkin untuk dilihat setelah pembaruan, jadi saya memutuskan untuk mengubah warna dalam pengaturan menjadi lebih terang. Pada titik ini saya terjebak dengan bug:
Anda tidak dapat mengubah warna tag !!! Lebih tepatnya, Anda bisa, tetapi editor tidak akan membiarkan Anda menyimpannya, merujuk pada nama yang sudah ada (WTF ???).
Gejala lain dari bug ini adalah tombol OK tidak aktif. Karena saya tidak dapat membuat perubahan pada tag nama yang sama, saya mencoba untuk mengubah namanya. Ya, ternyata Anda juga tidak bisa mengganti nama.
Akhirnya, Anda mungkin memperhatikan bahwa tema gelap tidak berfungsi untuk pengaturan, yang juga tidak terlalu bagus.
Setelah perjuangan panjang dengan sistem build di Windows saya akhirnya membangun Thunderbird dari file sumber. Versi terbaru dari klien surat ternyata jauh lebih baik daripada rilis baru. Di dalamnya, tema gelap sampai ke pengaturan juga, dan bug dengan editor tag ini menghilang. Namun demikian, untuk memastikan bahwa pembangunan proyek tidak hanya membuang-buang waktu, penganalisa kode statis
PVS-Studio mulai bekerja.
Catatan Kode sumber Thunderbird berpotongan dengan basis kode Firefox dengan beberapa cara. Oleh karena itu, analisis mencakup kesalahan dari komponen yang berbeda, yang patut dicermati oleh pengembang tim ini.
Catatan 2. Saat saya sedang menulis artikel, Thunderbird 68.1 dirilis dan bug ini diperbaiki:
comm
comm-central adalah repositori Mercurial dari kode ekstensi Thunderbird, SeaMonkey, dan Lightning.
V501 Ada sub-ekspresi identik '(! Strcmp (header, "Balas-Ke"))' di sebelah kiri dan di sebelah kanan '||' operator. nsEmitterUtils.cpp 28
extern "C" bool EmitThisHeaderForPrefSetting(int32_t dispType, const char *header) { .... if (nsMimeHeaderDisplayTypes::NormalHeaders == dispType) { if ((!strcmp(header, HEADER_DATE)) || (!strcmp(header, HEADER_TO)) || (!strcmp(header, HEADER_SUBJECT)) || (!strcmp(header, HEADER_SENDER)) || (!strcmp(header, HEADER_RESENT_TO)) || (!strcmp(header, HEADER_RESENT_SENDER)) || (!strcmp(header, HEADER_RESENT_FROM)) || (!strcmp(header, HEADER_RESENT_CC)) || (!strcmp(header, HEADER_REPLY_TO)) || (!strcmp(header, HEADER_REFERENCES)) || (!strcmp(header, HEADER_NEWSGROUPS)) || (!strcmp(header, HEADER_MESSAGE_ID)) || (!strcmp(header, HEADER_FROM)) || (!strcmp(header, HEADER_FOLLOWUP_TO)) || (!strcmp(header, HEADER_CC)) || (!strcmp(header, HEADER_ORGANIZATION)) || (!strcmp(header, HEADER_REPLY_TO)) || (!strcmp(header, HEADER_BCC))) return true; else return false; .... }
String
tajuk dibandingkan dengan konstanta
HEADER_REPLY_TO dua kali. Mungkin seharusnya ada konstanta lain di tempatnya.
V501 Ada sub-ekspresi identik 'obj-> options-> header! = MimeHeadersCitation' di sebelah kiri dan di sebelah kanan operator '&&'. mimemsig.cpp 536
static int MimeMultipartSigned_emit_child(MimeObject *obj) { .... if (obj->options && obj->options->headers != MimeHeadersCitation && obj->options->write_html_p && obj->options->output_fn && obj->options->headers != MimeHeadersCitation && sig->crypto_closure) { .... } .... }
Perbandingan aneh lain dari variabel dengan nama yang sama -
header . Seperti biasa, ada dua penjelasan yang memungkinkan: cek yang tidak perlu atau salah ketik.
V517 Penggunaan
pola 'jika (A) {...} else jika (A) {...}' terdeteksi. Ada kemungkinan kehadiran kesalahan logis. Periksa baris: 1306, 1308. MapiApi.cpp 1306
void CMapiApi::ReportLongProp(const char *pTag, LPSPropValue pVal) { if (pVal && (PROP_TYPE(pVal->ulPropTag) == PT_LONG)) { nsCString num; nsCString num2; num.AppendInt((int32_t)pVal->Value.l); num2.AppendInt((int32_t)pVal->Value.l, 16); MAPI_TRACE3("%s %s, 0x%s\n", pTag, num, num2); } else if (pVal && (PROP_TYPE(pVal->ulPropTag) == PT_NULL)) { MAPI_TRACE1("%s {NULL}\n", pTag); } else if (pVal && (PROP_TYPE(pVal->ulPropTag) == PT_ERROR)) {
Ctrl + C dan Ctrl + V kunci pasti membantu mempercepat penulisan kaskade ekspresi kondisional ini. Akibatnya, salah satu cabang tidak akan pernah dieksekusi.
V517 Penggunaan
pola 'jika (A) {...} else jika (A) {...}' terdeteksi. Ada kemungkinan kehadiran kesalahan logis. Periksa baris: 777, 816. nsRDFContentSink.cpp 777
nsresult RDFContentSinkImpl::GetIdAboutAttribute(const char16_t** aAttributes, nsIRDFResource** aResource, bool* aIsAnonymous) { .... if (localName == nsGkAtoms::about) { .... } else if (localName == nsGkAtoms::ID) { .... } else if (localName == nsGkAtoms::nodeID) { nodeID.Assign(aAttributes[1]); } else if (localName == nsGkAtoms::about) {
Kondisi pertama dan terakhir adalah sama. Kode tersebut menunjukkan bahwa ia masih dalam proses penulisan. Dapat dengan aman dikatakan bahwa kesalahan akan muncul dengan sendirinya setelah kode disempurnakan. Seorang programmer dapat mengubah kode yang dikomentari, tetapi tidak akan pernah bisa mengendalikannya. Tolong, hati-hati dan penuh perhatian dengan kode ini.
V522 Dereferencing dari 'baris' penunjuk nol mungkin terjadi. morkRowCellCursor.cpp 175
NS_IMETHODIMP morkRowCellCursor::MakeCell(
Kemungkinan dereference dari pointer null
baris di baris berikut:
morkCell* cell = row->CellAt(ev, pos);
Kemungkinan besar, pointer tidak diinisialisasi, misalnya, dengan metode
GetRow , dll.
V543 Aneh bahwa nilai '-1' ditugaskan ke variabel 'm_lastError' tipe HRESULT. MapiApi.cpp 1050
class CMapiApi { .... private: static HRESULT m_lastError; .... }; CMsgStore *CMapiApi::FindMessageStore(ULONG cbEid, LPENTRYID lpEid) { if (!m_lpSession) { MAPI_TRACE0("FindMessageStore called before session is open\n"); m_lastError = -1; return NULL; } .... }
Tipe
HRESULT adalah tipe data yang kompleks. Bitnya yang berbeda mewakili bidang yang berbeda dari deskripsi kesalahan. Anda perlu mengatur kode kesalahan menggunakan konstanta khusus dari file header sistem.
Beberapa fragmen seperti ini:
- V543 Aneh bahwa nilai '-1' ditugaskan ke variabel 'm_lastError' tipe HRESULT. MapiApi.cpp 817
- V543 Aneh bahwa nilai '-1' ditugaskan ke variabel 'm_lastError' tipe HRESULT. MapiApi.cpp 1749
V579 Fungsi memset menerima pointer dan ukurannya sebagai argumen. Itu mungkin sebuah kesalahan. Periksa argumen ketiga. icalmime.c 195
icalcomponent* icalmime_parse(....) { struct sspm_part *parts; int i, last_level=0; icalcomponent *root=0, *parent=0, *comp=0, *last = 0; if ( (parts = (struct sspm_part *) malloc(NUM_PARTS*sizeof(struct sspm_part)))==0) { icalerror_set_errno(ICAL_NEWFAILED_ERROR); return 0; } memset(parts,0,sizeof(parts)); sspm_parse_mime(parts, NUM_PARTS, icalmime_local_action_map, get_string, data, 0 ); .... }
Variabel
bagian adalah penunjuk ke array struktur. Untuk mereset nilai struktur, penulis menggunakan fungsi
memset , tetapi melewati ukuran pointer sebagai ukuran ruang memori.
Fragmen mencurigakan yang serupa:
- V579 Fungsi memset menerima pointer dan ukurannya sebagai argumen. Itu mungkin sebuah kesalahan. Periksa argumen ketiga. icalmime.c 385
- V579 Fungsi memset menerima pointer dan ukurannya sebagai argumen. Itu mungkin sebuah kesalahan. Periksa argumen ketiga. icalparameter.c 114
- V579 Fungsi snprintf menerima pointer dan ukurannya sebagai argumen. Itu mungkin sebuah kesalahan. Periksa argumen kedua. icaltimezone.c 1908
- V579 Fungsi snprintf menerima pointer dan ukurannya sebagai argumen. Itu mungkin sebuah kesalahan. Periksa argumen kedua. icaltimezone.c 1910
- V579 Fungsi strncmp menerima pointer dan ukurannya sebagai argumen. Itu mungkin sebuah kesalahan. Periksa argumen ketiga. sspm.c 707
- V579 Fungsi strncmp menerima pointer dan ukurannya sebagai argumen. Itu mungkin sebuah kesalahan. Periksa argumen ketiga. sspm.c 813
V595 Pointer 'aValues' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 553, 555. nsLDAPMessage.cpp 553
NS_IMETHODIMP nsLDAPMessage::GetBinaryValues(const char *aAttr, uint32_t *aCount, nsILDAPBERValue ***aValues) { .... *aValues = static_cast<nsILDAPBERValue **>( moz_xmalloc(numVals * sizeof(nsILDAPBERValue))); if (!aValues) { ldap_value_free_len(values); return NS_ERROR_OUT_OF_MEMORY; } .... }
Diagnostik
V595 biasanya mendeteksi kesalahan tipikal dari dereference pointer nol. Dalam hal ini kami memiliki contoh yang sangat menarik, layak mendapat perhatian khusus.
Secara teknis penganalisa benar bahwa pointer
aValues pertama kali
direferensikan dan kemudian diperiksa, tetapi kesalahan sebenarnya berbeda. Ini adalah penunjuk ganda, jadi kode yang benar akan terlihat sebagai berikut:
*aValues = static_cast<nsILDAPBERValue **>( moz_xmalloc(numVals * sizeof(nsILDAPBERValue))); if (!*aValues) { ldap_value_free_len(values); return NS_ERROR_OUT_OF_MEMORY; }
Fragmen lain yang serupa:
- V595 Pointer '_retval' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 357, 358. nsLDAPSyncQuery.cpp 357
V1044 Kondisi break loop tidak tergantung pada jumlah iterasi. mimemoz2.cpp 1795
void ResetChannelCharset(MimeObject *obj) { .... if (cSet) { char *ptr2 = cSet; while ((*cSet) && (*cSet != ' ') && (*cSet != ';') && (*cSet != '\r') && (*cSet != '\n') && (*cSet != '"')) ptr2++; if (*cSet) { PR_FREEIF(obj->options->default_charset); obj->options->default_charset = strdup(cSet); obj->options->override_charset = true; } PR_FREEIF(cSet); } .... }
Kesalahan ini ditemukan menggunakan diagnostik baru yang akan tersedia di rilis penganalisa berikutnya. Semua variabel yang digunakan dalam kondisi
while tidak berubah, karena variabel
ptr2 dan
cSet bingung dalam isi fungsi.
netwerk
netwerk berisi antarmuka C dan kode untuk akses tingkat rendah ke jaringan (menggunakan soket dan cache file dan memori) serta akses tingkat lebih tinggi (menggunakan berbagai protokol seperti http, ftp, gopher, castanet). Kode ini juga dikenal dengan nama, "netlib" dan "Necko."
V501 Ada sub-ekspresi identik 'connectStarted' ke kiri dan ke kanan operator '&&'. nsSocketTransport2.cpp 1693
nsresult nsSocketTransport::InitiateSocket() { .... if (gSocketTransportService->IsTelemetryEnabledAndNotSleepPhase() && connectStarted && connectCalled) {
Pertama saya berpikir bahwa menduplikasi variabel
connectStarted hanyalah kode yang berlebihan. Tapi kemudian saya melihat seluruh fungsi yang cukup panjang dan menemukan sebuah fragmen yang serupa. Kemungkinan besar, variabel
connectCalled harus ada di sini alih-
alih variabel
connectStarted .
V611 Memori dialokasikan menggunakan operator 'T baru] tetapi dirilis menggunakan operator' hapus '. Pertimbangkan untuk memeriksa kode ini. Mungkin lebih baik menggunakan 'delete [] mData;'. Periksa baris: 233, 222. DataChannel.cpp 233
BufferedOutgoingMsg::BufferedOutgoingMsg(OutgoingMsg& msg) { size_t length = msg.GetLeft(); auto* tmp = new uint8_t[length];
Pointer
mData menunjuk ke array, bukan objek tunggal. Kesalahan dibuat di destruktor kelas karena tanda kurung yang hilang untuk operator
hapus .
V1044 Kondisi break loop tidak tergantung pada jumlah iterasi. ParseFTPList.cpp 691
int ParseFTPList(....) { .... pos = toklen[2]; while (pos > (sizeof(result->fe_size) - 1)) pos = (sizeof(result->fe_size) - 1); memcpy(result->fe_size, tokens[2], pos); result->fe_size[pos] = '\0'; .... }
Nilai variabel
pos akan ditulis ulang dalam loop untuk nilai yang sama. Sepertinya diagnostik baru telah menemukan kesalahan lain.
gfx
gfx berisi antarmuka C dan kode untuk platform independent drawing and imaging. Dapat digunakan untuk menggambar persegi panjang, garis, gambar, dll. Pada dasarnya, ini adalah satu set antarmuka untuk konteks perangkat (platform) independen platform. Itu tidak menangani widget atau rutinitas menggambar tertentu; itu hanya memberikan operasi primitif untuk menggambar.
V501 Ada sub-ekspresi identik ke kiri dan ke kanan '||' operator: mVRSistem || mVRKomposisi || mVRSystem OpenVRSession.cpp 876
void OpenVRSession::Shutdown() { StopHapticTimer(); StopHapticThread(); if (mVRSystem || mVRCompositor || mVRSystem) { ::vr::VR_Shutdown(); mVRCompositor = nullptr; mVRChaperone = nullptr; mVRSystem = nullptr; } }
Variabel
mVRSystem muncul dalam kondisi dua kali
. Jelas, salah satu kemunculannya harus diganti dengan
mVRChaperone.dom
dom berisi antarmuka C dan kode untuk mengimplementasikan dan melacak objek DOM (Document Object Model) dalam Javascript. Ini membentuk substruktur C yang menciptakan, menghancurkan dan memanipulasi objek bawaan dan yang ditentukan pengguna sesuai dengan skrip Javascript.
V570 Variabel 'clonedDoc-> mPreloadReferrerInfo' ditugaskan untuk dirinya sendiri. Document.cpp 12049
already_AddRefed<Document> Document::CreateStaticClone( nsIDocShell* aCloneContainer) { .... clonedDoc->mReferrerInfo = static_cast<dom::ReferrerInfo*>(mReferrerInfo.get())->Clone(); clonedDoc->mPreloadReferrerInfo = clonedDoc->mPreloadReferrerInfo; .... }
Penganalisa menemukan penugasan variabel untuk dirinya sendiri.
xpcom
xpcom berisi antarmuka C tingkat rendah, kode C, kode C, sedikit kode rakitan dan alat baris perintah untuk mengimplementasikan mesin dasar komponen XPCOM (yang merupakan singkatan dari "Cross Platform Component Object Model"). XPCOM adalah mekanisme yang memungkinkan Mozilla untuk mengekspor antarmuka dan membuatnya tersedia secara otomatis untuk skrip JavaScript, Microsoft COM dan kode Mozilla C biasa.
V611 Memori dialokasikan menggunakan fungsi 'malloc / realloc' tetapi dirilis menggunakan operator 'hapus'. Pertimbangkan untuk memeriksa logika operasi di belakang variabel 'kunci'. Periksa baris: 143, 140. nsINIParser.h 143
struct INIValue { INIValue(const char* aKey, const char* aValue) : key(strdup(aKey)), value(strdup(aValue)) {} ~INIValue() { delete key; delete value; } void SetValue(const char* aValue) { delete value; value = strdup(aValue); } const char* key; const char* value; mozilla::UniquePtr<INIValue> next; };
Setelah memanggil fungsi
strdup , seseorang harus melepaskan memori menggunakan fungsi
bebas , bukan operator
hapus .
V716 Konversi tipe mencurigakan dalam inisialisasi: 'HRESULT var = BOOL'. SpecialSystemDirectory.cpp 73
BOOL SHGetSpecialFolderPathW( HWND hwnd, LPWSTR pszPath, int csidl, BOOL fCreate ); static nsresult GetWindowsFolder(int aFolder, nsIFile** aFile) { WCHAR path_orig[MAX_PATH + 3]; WCHAR* path = path_orig + 1; HRESULT result = SHGetSpecialFolderPathW(nullptr, path, aFolder, true); if (!SUCCEEDED(result)) { return NS_ERROR_FAILURE; } .... }
SHGetSpecialFolderPathW Fungsi WinAPI mengembalikan nilai tipe
BOOL , bukan
HRESULT . Kita harus menulis ulang cek hasil fungsi ke yang benar.
nsprpub
nsprpub berisi kode C untuk lintas platform "C" Runtime Library. Perpustakaan Runtime āCā berisi fungsi-fungsi C non-visual dasar untuk mengalokasikan dan membatalkan alokasi memori, mendapatkan waktu dan tanggal, membaca dan menulis file, menangani utas dan menangani serta membandingkan string di semua platform
V647 Nilai tipe 'int' ditugaskan ke pointer tipe 'pendek'. Pertimbangkan untuk memeriksa tugas: 'out_flags = 0x2'. prsocket.c 1220
#define PR_POLL_WRITE 0x2 static PRInt16 PR_CALLBACK SocketPoll( PRFileDesc *fd, PRInt16 in_flags, PRInt16 *out_flags) { *out_flags = 0; #if defined(_WIN64) if (in_flags & PR_POLL_WRITE) { if (fd->secret->alreadyConnected) { out_flags = PR_POLL_WRITE; return PR_POLL_WRITE; } } #endif return in_flags; }
Penganalisis telah mendeteksi menetapkan konstanta numerik ke pointer
out_flags . Kemungkinan besar, seseorang baru saja lupa untuk melakukannya:
if (fd->secret->alreadyConnected) { *out_flags = PR_POLL_WRITE; return PR_POLL_WRITE; }
Kesimpulan
Ini belum berakhir. Biarkan ulasan kode baru! Kode Thunderbird dan Firefox terdiri dari dua perpustakaan besar: Layanan Keamanan Jaringan (NSS) dan WebRTC (Web Real Time Communications). Saya menemukan beberapa kesalahan yang meyakinkan. Dalam ulasan ini saya akan menunjukkan satu dari setiap proyek.
NssV597 Kompiler dapat menghapus pemanggilan fungsi 'memset', yang digunakan untuk membersihkan buffer 'newdeskey'. Fungsi RtlSecureZeroMemory () harus digunakan untuk menghapus data pribadi. pkcs11c.c 1033
static CK_RV sftk_CryptInit(....) { .... unsigned char newdeskey[24]; .... context->cipherInfo = DES_CreateContext( useNewKey ? newdeskey : (unsigned char *)att->attrib.pValue, (unsigned char *)pMechanism->pParameter, t, isEncrypt); if (useNewKey) memset(newdeskey, 0, sizeof newdeskey); sftk_FreeAttribute(att); .... }
NSS adalah perpustakaan untuk mengembangkan aplikasi klien dan server yang aman. Sedangkan DES Key tidak dihapus di sini. Kompiler akan menghapus panggilan
memset dari kode, karena array
kunci baru tidak digunakan di mana pun dalam kode lebih lanjut.
WebRTCV519 Variabel 'state [state_length - x_length + i]' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 83, 84. filter_ar.c 84
size_t WebRtcSpl_FilterAR(....) { .... for (i = 0; i < state_length - x_length; i++) { state[i] = state[i + x_length]; state_low[i] = state_low[i + x_length]; } for (i = 0; i < x_length; i++) { state[state_length - x_length + i] = filtered[i]; state[state_length - x_length + i] = filtered_low[i];
Pada loop kedua, data ditulis dalam array yang salah, karena penulis menyalin kode dan lupa mengubah nama array
negara untuk
state_low .
Mungkin, masih ada bug menarik dalam proyek ini, yang harus diceritakan. Dan kami akan segera melakukannya. Sementara itu, cobalah
PVS-Studio di proyek Anda.