بدأت المغامرات مع عميل بريد Mozilla Thunderbird بالتحديث التلقائي إلى الإصدار 68.0. المزيد من النص في الإشعارات المنبثقة والسمة المظلمة الافتراضية هي السمات البارزة لهذا الإصدار. في بعض الأحيان ، وجدت خطأً كنت أتوق إليه على الفور بالكشف عن التحليل الثابت. أصبح هذا هو السبب للذهاب لفحص آخر من شفرة المصدر للمشروع باستخدام PVS-Studio. لقد حدث أنه بحلول وقت التحليل ، تم إصلاح الخلل بالفعل. ومع ذلك ، نظرًا لأننا أولينا بعض الاهتمام بالمشروع ، فلا يوجد سبب لعدم الكتابة عن العيوب الأخرى التي تم العثور عليها.
مقدمة
المظهر المظلم لإصدار ثندربيرد الجديد يبدو جميلاً. أنا أحب الموضوعات الظلام. لقد قمت بالفعل بالتبديل إليهم في برامج messagers و Windows و macOS. قريبا سيتم تحديث iPhone إلى iOS 13 مع موضوع مظلم. لهذا السبب اضطررت حتى إلى تغيير جهاز iPhone 5S لطراز أحدث. في الممارسة العملية ، اتضح أن السمة الداكنة تتطلب المزيد من الجهد للمطورين لالتقاط ألوان الواجهة. لا يمكن للجميع التعامل معها في المرة الأولى.
هذه هي الطريقة التي تبدو بها العلامات القياسية في ثندربيرد بعد التحديث:
عادةً ما أستخدم 6 علامات (5 تخصيص قياسي 1+) لترميز رسائل البريد الإلكتروني. أصبح نصفهم من المستحيل النظر إلى التحديث ، لذلك قررت تغيير اللون في الإعدادات إلى أكثر إشراقًا. في هذه المرحلة ، تعثرت مع خلل:
لا يمكنك تغيير لون العلامة! يمكنك حقًا ، ولكن المحرر لن يسمح لك بحفظه ، في إشارة إلى اسم موجود بالفعل (WTF ؟؟؟).
آخر أعراض هذا الخطأ هو زر موافق غير نشط. نظرًا لأنني لم أستطع إجراء تغييرات في علامة الاسم نفسها ، حاولت تغيير اسمها. حسنًا ، اتضح أنه لا يمكنك إعادة تسميته أيضًا.
أخيرًا ، ربما لاحظت أن السمة المظلمة لا تعمل مع الإعدادات ، وهي أيضًا ليست لطيفة جدًا.
بعد صراع طويل مع نظام الإنشاء في Windows ، قمت في النهاية بتصميم Thunderbird من الملفات المصدر. تبين أن أحدث إصدار من عميل البريد أفضل بكثير من الإصدار الجديد. في ذلك ، وصل المظهر المظلم إلى الإعدادات أيضًا ، واختفى هذا الخطأ مع محرر العلامات. ومع ذلك ، لضمان أن مبنى المشروع ليس مجرد مضيعة للوقت ، فإن محلل الشفرة الثابتة في
PVS-Studio بدأ عمله.
المذكرة. تتقاطع شفرة مصدر Thunderbird مع قاعدة كود Firefox ببعض الوسائل. لذلك ، يتضمن التحليل أخطاء من مكونات مختلفة ، والتي تستحق نظرة فاحصة من قبل مطوري هذه الفرق.
ملاحظة 2. أثناء كتابة المقالة ، تم إصدار Thunderbird 68.1 وتم إصلاح هذا الخطأ:
بالاتصالات
comm-central هو مستودع للزئبق لرمز امتداد Thunderbird و SeaMonkey و Lightning.
V501 هناك تعبيرات فرعية متطابقة '(! Strcmp (الرأس ، "Reply-To"))' إلى اليسار وإلى يمين '||' المشغل. 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; .... }
تمت مقارنة سلسلة
الرأس مع ثابت
HEADER_REPLY_TO مرتين. ربما كان ينبغي أن يكون هناك ثابت آخر في مكانه.
V501 هناك تعبيرات فرعية متطابقة "obj-> options-> headers! = MimeHeadersCitation" إلى اليسار وإلى يمين عامل التشغيل &&. 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) { .... } .... }
مقارنة أخرى غريبة لمتغير يحمل نفس الاسم -
الرؤوس . كما هو الحال دائمًا ، هناك تفسيران محتملان: فحص غير ضروري أو خطأ مطبعي.
V517 استخدام
نمط "if (A) {...} آخر إذا تم اكتشاف (A) {...}". هناك احتمال لوجود خطأ منطقي. خطوط الفحص: 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 و Ctrl + V في تسريع كتابة سلسلة التعابير الشرطية هذه. نتيجة لذلك ، لن يتم تنفيذ أحد الفروع.
V517 استخدام
نمط "if (A) {...} آخر إذا تم اكتشاف (A) {...}". هناك احتمال لوجود خطأ منطقي. خطوط التحقق: 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) {
الشرط الأول والأخير هي نفسها. يظهر الرمز أنه لا يزال في طور الكتابة. قد يقال بأمان أن الخطأ سيظهر نفسه بعد تنقيح الكود. يمكن للمبرمج تغيير التعليمات البرمجية التي تم التعليق عليها ، ولكن لن يتمكن من التحكم فيها. من فضلك ، كن حذرا للغاية ويقظة مع هذا الرمز.
V522 قد يتم إلغاء تحديد "صف" المؤشر الخالي. morkRowCellCursor.cpp 175
NS_IMETHODIMP morkRowCellCursor::MakeCell(
احتمال dereference من مؤشر
صف فارغ في السطر التالي:
morkCell* cell = row->CellAt(ev, pos);
على الأرجح ، لم يتم تهيئة المؤشر ، على سبيل المثال ، بواسطة أسلوب
GetRow ، إلخ.
V543 من الغريب أن يتم تعيين القيمة "-1" للمتغير "m_lastError" من نوع 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; } .... }
نوع
HRESULT هو نوع بيانات معقد. تمثل وحدات البت المختلفة حقولًا مختلفة لوصف الخطأ. تحتاج إلى تعيين رمز الخطأ باستخدام ثوابت خاصة من ملفات رأس النظام.
بضع شظايا مثل هذا:
- V543 من الغريب أن يتم تعيين القيمة "-1" للمتغير "m_lastError" من نوع HRESULT. MapiApi.cpp 817
- V543 من الغريب أن يتم تعيين القيمة "-1" للمتغير "m_lastError" من نوع HRESULT. MapiApi.cpp 1749
V579 تتلقى الدالة memset المؤشر وحجمها كوسائط. ربما يكون خطأ. تفقد الحجة الثالثة. 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 ); .... }
متغير
أجزاء مؤشر إلى مجموعة من الهياكل. لإعادة ضبط قيم الهياكل ، استخدم المؤلفون وظيفة
memset ، لكنهم تجاوزوا حجم المؤشر كحجم مساحة الذاكرة.
شظايا مشبوهة مماثلة:
- V579 تتلقى الدالة memset المؤشر وحجمها كوسائط. ربما يكون خطأ. تفقد الحجة الثالثة. icalmime.c 385
- V579 تتلقى الدالة memset المؤشر وحجمها كوسائط. ربما يكون خطأ. تفقد الحجة الثالثة. icalparameter.c 114
- V579 تتلقى الدالة snprintf المؤشر وحجمها كوسائط. ربما يكون خطأ. تفقد الحجة الثانية. icaltimezone.c 1908
- V579 تتلقى الدالة snprintf المؤشر وحجمها كوسائط. ربما يكون خطأ. تفقد الحجة الثانية. icaltimezone.c 1910
- V579 تتلقى الدالة strncmp المؤشر وحجمها كوسائط. ربما يكون خطأ. تفقد الحجة الثالثة. sspm.c 707
- V579 تتلقى الدالة strncmp المؤشر وحجمها كوسائط. ربما يكون خطأ. تفقد الحجة الثالثة. sspm.c 813
V595 تم استخدام مؤشر "aValues" قبل أن يتم التحقق منه ضد nullptr. خطوط التحقق: 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; } .... }
تشخيص
V595 عادةً ما يكشف الأخطاء النموذجية dereference مؤشر فارغة. في هذه الحالة لدينا مثال مثير للاهتمام للغاية ، يستحق عناية خاصة.
من الناحية الفنية ، يكون المحلل على
حق في أن مؤشر
aValues يتم
إلغاء ترجمته أولاً ثم يتم التحقق منه ، لكن الخطأ الفعلي مختلف. إنه مؤشر مزدوج ، لذلك يجب أن يبدو الرمز الصحيح كما يلي:
*aValues = static_cast<nsILDAPBERValue **>( moz_xmalloc(numVals * sizeof(nsILDAPBERValue))); if (!*aValues) { ldap_value_free_len(values); return NS_ERROR_OUT_OF_MEMORY; }
جزء آخر مماثل:
- V595 تم استخدام المؤشر '_retval' قبل أن يتم التحقق منه مقابل nullptr. خطوط التحقق: 357 ، 358. nsLDAPSyncQuery.cpp 357
لا تعتمد شروط فاصل الحلقة
V1044 على عدد التكرارات. 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); } .... }
تم العثور على هذا الخطأ باستخدام تشخيص جديد سيكون متاحًا في الإصدار التالي للمحلل. لا تتغير جميع المتغيرات المستخدمة في حالة حلقة
أثناء ، حيث يتم خلط المتغيرات
ptr2 و
cSet في نص الوظيفة.
netwerk
يحتوي netwerk على واجهات ورمز C للوصول المنخفض إلى الشبكة (باستخدام مآخذ وذاكرة التخزين المؤقت للملفات والذاكرة) بالإضافة إلى الوصول إلى المستوى الأعلى (باستخدام بروتوكولات مختلفة مثل http ، ftp ، gopher ، castanet). يُعرف هذا الرمز أيضًا باسم "netlib" و "Necko".
V501 هناك تعبيرات فرعية مماثلة "connectStarted" إلى اليسار وإلى يمين المشغل "&&". nsSocketTransport2.cpp 1693
nsresult nsSocketTransport::InitiateSocket() { .... if (gSocketTransportService->IsTelemetryEnabledAndNotSleepPhase() && connectStarted && connectCalled) {
أولاً اعتقدت أن تكرار متغير
connectStarted هو مجرد رمز زائدة عن الحاجة. ولكن بعد ذلك نظرت من خلال الوظيفة الطويلة جدًا ووجدت جزءًا مشابهًا. على الأرجح ، يجب أن يكون متغير
connectCalled هنا بدلاً من متغير
connectStarted .
V611 تم تخصيص الذاكرة باستخدام عامل التشغيل "T []" الجديد ولكن تم إصدارها باستخدام عامل التشغيل "delete". النظر في فحص هذا الرمز. من الأفضل استخدام "delete [] mData؛". خطوط التحقق: 233 ، 222. DataChannel.cpp 233
BufferedOutgoingMsg::BufferedOutgoingMsg(OutgoingMsg& msg) { size_t length = msg.GetLeft(); auto* tmp = new uint8_t[length];
يشير مؤشر
mData إلى صفيف ، وليس كائن واحد. حدث خطأ في destructor الفئة بسبب الأقواس المفقودة لمشغل
الحذف .
لا تعتمد شروط فاصل الحلقة
V1044 على عدد التكرارات. 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'; .... }
يتم إعادة كتابة قيمة متغير
pos في الحلقة بنفس القيمة. يبدو أن التشخيص الجديد وجد خطأً آخر.
GFX
يحتوي gfx على واجهات C ورمز للرسم والتصوير المستقل للمنصة. يمكن استخدامه لرسم المستطيلات والخطوط والصور بشكل أساسي ، هي مجموعة من الواجهات لسياق (رسم) جهاز مستقل عن النظام الأساسي. لا يتعامل مع الحاجيات أو إجراءات الرسم المحددة ؛ يوفر فقط العمليات البدائية للرسم.
V501 هناك تعبيرات فرعية مماثلة إلى اليسار وإلى يمين "||" المشغل: mVRSystem || mVRCompositor || mVRSystem OpenVRSession.cpp 876
void OpenVRSession::Shutdown() { StopHapticTimer(); StopHapticThread(); if (mVRSystem || mVRCompositor || mVRSystem) { ::vr::VR_Shutdown(); mVRCompositor = nullptr; mVRChaperone = nullptr; mVRSystem = nullptr; } }
يظهر متغير
mVRSystem في الحالة مرتين
. من الواضح ، يجب استبدال واحدة من الحوادث مع
mVRChaperone.دوم
يحتوي dom على واجهات ورمز C لتطبيق وتتبع كائنات DOM (طراز كائن المستند) في Javascript. إنه يشكل البنية التحتية C التي تقوم بإنشاء وتدمير ومعالجة الكائنات المضمنة والمعرّفة من قِبل المستخدم وفقًا للبرنامج النصي Javascript.
V570 يتم تعيين المتغير 'clonedDoc-> mPreloadReferrerInfo' لنفسه. Document.cpp 12049
already_AddRefed<Document> Document::CreateStaticClone( nsIDocShell* aCloneContainer) { .... clonedDoc->mReferrerInfo = static_cast<dom::ReferrerInfo*>(mReferrerInfo.get())->Clone(); clonedDoc->mPreloadReferrerInfo = clonedDoc->mPreloadReferrerInfo; .... }
وجد المحلل تعيين المتغير لنفسه.
XPCOM
يحتوي xpcom على واجهات C منخفضة المستوى ، C رمز ، C رمز ، قليلا من رمز التجميع وأدوات سطر الأوامر لتنفيذ الآلية الأساسية لمكونات XPCOM (التي تقف على "طراز كائن النظام الأساسي للمكونات"). XPCOM هي الآلية التي تسمح لـ Mozilla بتصدير الواجهات وجعلها متوفرة تلقائيًا لنصوص JavaScript ، ول Microsoft COM ولرمز Mozilla C العادي.
V611 تم تخصيص الذاكرة باستخدام وظيفة "malloc / realloc" ولكن تم إصدارها باستخدام عامل التشغيل "delete". النظر في فحص منطق العملية وراء المتغير "مفتاح". خطوط التحقق: 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; };
بعد استدعاء وظيفة
strdup ، يتعين على المرء تحرير الذاكرة باستخدام الوظيفة
المجانية ، وليس مشغل
الحذف .
V716 تحويل النوع المشبوه في التهيئة: '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 WinAPI تقوم بإرجاع قيمة نوع
BOOL ، وليس
HRESULT . على المرء أن يعيد كتابة التحقق من وظيفة النتيجة إلى الصحيح.
nsprpub
يحتوي nsprpub على رمز C لـ Runtime Library "C" عبر النظام الأساسي. تحتوي مكتبة وقت تشغيل "C" على وظائف C غير مرئية أساسية لتخصيص الذاكرة وإلغاء تخصيصها ، والحصول على الوقت والتاريخ ، وقراءة الملفات وكتابتها ، والتعامل مع مؤشرات الترابط ، والتعامل مع السلاسل ومقارنتها عبر جميع الأنظمة الأساسية
V647 يتم
تعيين قيمة النوع "int" إلى مؤشر النوع "short". ضع في اعتبارك فحص المهمة: '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; }
اكتشف المحلل تعيين ثابت
رقمي لمؤشر
out_flags . على الأرجح ، لقد نسي المرء مجرد التراجع:
if (fd->secret->alreadyConnected) { *out_flags = PR_POLL_WRITE; return PR_POLL_WRITE; }
استنتاج
انها ليست النهاية بعد. دع مراجعات الكود الجديد تكون! يتكون كود Thunderbird و Firefox من مكتبتين كبيرتين: خدمات أمان الشبكات (NSS) و WebRTC (اتصالات الويب في الوقت الفعلي). لقد وجدت هناك بعض الأخطاء القهرية. في هذا الاستعراض سوف تظهر واحدة من كل مشروع.
NSSV597 يمكن للمترجم حذف استدعاء دالة "memset" ، والذي يستخدم لمسح المخزن المؤقت "newdeskey". يجب استخدام الدالة RtlSecureZeroMemory () لمسح البيانات الخاصة. 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 هي مكتبة لتطوير تطبيقات العميل والخادم آمنة. بينما لم يتم مسح DES Key هنا. سيقوم المحول البرمجي بحذف استدعاء
memset من الكود ، حيث لا يتم استخدام مصفوفة
newdeskey في أي مكان في الكود.
يتطلب WebRTCV519 يتم تعيين القيم "state [state_length - x_length + i]" لقيمتين متتاليتين. ربما هذا خطأ. خطوط التحقق: 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];
في الحلقة الثانية ، تتم كتابة البيانات في صفيف خاطئ ، لأن المؤلف قام بنسخ الكود ونسي تغيير اسم مصفوفة
الحالة لـ
state_low .
ربما ، لا يزال هناك أخطاء مثيرة للاهتمام في هذه المشاريع ، والتي ينبغي أن يقال عنها. ونحن سوف نفعل ذلك قريبا. في غضون ذلك ، جرب
PVS-Studio في مشروعك.