雷鸟的黑暗主题是运行代码分析器的原因

Mozilla Thunderbird邮件客户端的冒险始于自动更新到版本68.0。 弹出通知中的更多文本和默认的深色主题是此版本的显着功能。 有时我发现一个错误，我立即渴望通过静态分析进行检测。 这就是使用PVS-Studio再次检查项目源代码的原因。 碰巧的是，在进行分析时，该错误已得到修复。 但是，由于我们已经对该项目给予了一定的关注，因此没有理由不撰写其他发现的缺陷。

引言

新Thunderbird版本的深色主题看起来很漂亮。 我喜欢黑暗的主题。 我已经在Messenger，Windows，macOS中切换到它们。 iPhone很快就会以深色主题更新到iOS 13。 因此，我什至不得不将iPhone 5S换成较新的型号。 实际上，事实证明，深色主题需要开发人员花更多的精力来选择界面的颜色。 并非每个人都能在第一时间处理它。 更新后，这是标准标签在Thunderbird中的样子：

我通常使用6个标签（5个标准+1自定义标签）来标记电子邮件。 更新后，其中的一半变得无法查看，因此我决定将设置中的颜色更改为更亮的颜色。 此时，我陷入了一个错误：

您不能更改标签颜色！ 更确切地说，您可以，但是编辑器不允许您使用已存在的名称（WTF ???）进行保存。

此错误的另一个症状是无效的“确定”按钮。 由于无法在相同的名称标签中进行更改，因此我尝试更改其名称。 好吧，事实证明您也不能重命名。

最后，您可能已经注意到，深色主题不适用于设置，这也不是很好。

经过与Windows中的构建系统的长期斗争之后，我最终从源文件构建了Thunderbird。 事实证明，最新版本的邮件客户端比新版本要好得多。 在其中，暗色主题也进入了设置，并且带有标签编辑器的错误消失了。 尽管如此，为了确保项目的建设不只是浪费时间， PVS-Studio静态代码分析器已开始工作。

注意事项 Thunderbird的源代码在某种程度上与Firefox代码库相交。 因此，分析包括来自不同组件的错误，这些团队的开发人员值得仔细研究。

注意2.在撰写本文时，Thunderbird 68.1已发布，并且此错误已修复：

通讯

comm-central是Thunderbird，SeaMonkey和Lightning扩展代码的Mercurial存储库。

V501在'||'的左右两侧有相同的子表达式'（！Strcmp（header，“ 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）{...} else if（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)) { // <= MAPI_TRACE1("%s {Error retrieving property}\n", pTag); } else if (pVal && (PROP_TYPE(pVal->ulPropTag) == PT_ERROR)) { // <= MAPI_TRACE1("%s {Error retrieving property}\n", pTag); } else { MAPI_TRACE1("%s invalid value, expecting long\n", pTag); } if (pVal) MAPIFreeBuffer(pVal); } 

Ctrl + C和Ctrl + V键无疑有助于加快条件表达式级联的编写速度。 结果，分支之一将永远不会执行。

V517检测到使用'if（A）{...} else if（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) { // XXX we don't deal with aboutEach... //MOZ_LOG(gLog, LogLevel::Warning, // ("rdfxml: ignoring aboutEach at line %d", // aNode.GetSourceLineNumber())); } .... } 

第一个条件和最后一个条件相同。 该代码表明它仍在编写过程中。 可以肯定地说，错误将在代码完善后显示出来。 程序员可以更改注释的代码，但永远无法控制它。 请注意此代码。

V522可能会取消引用空指针“行”。 第175章

 NS_IMETHODIMP morkRowCellCursor::MakeCell( // get cell at current pos in the row nsIMdbEnv* mev, // context mdb_column* outColumn, // column for this particular cell mdb_pos* outPos, // position of cell in row sequence nsIMdbCell** acqCell) { nsresult outErr = NS_OK; nsIMdbCell* outCell = 0; mdb_pos pos = 0; mdb_column col = 0; morkRow* row = 0; morkEnv* ev = morkEnv::FromMdbEnv(mev); if (ev) { pos = mCursor_Pos; morkCell* cell = row->CellAt(ev, pos); if (cell) { col = cell->GetColumn(); outCell = row->AcquireCellHandle(ev, cell, col, pos); } outErr = ev->AsErr(); } if (acqCell) *acqCell = outCell; if (outPos) *outPos = pos; if (outColumn) *outColumn = col; return outErr; } 

在以下行中可能会取消引用行空指针：

 morkCell* cell = row->CellAt(ev, pos); 

很有可能未通过GetRow方法等初始化指针。

V543将值“ -1”分配给HRESULT类型的变量“ m_lastError”是很奇怪的。 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”分配给HRESULT类型的变量“ m_lastError”是很奇怪的。 MapiApi.cpp 817
• V543将值“ -1”分配给HRESULT类型的变量“ m_lastError”是很奇怪的。 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, /* Max parts */ icalmime_local_action_map, /* Actions */ get_string, data, /* data for get_string*/ 0 /* First header */); .... } 

parts变量是指向结构数组的指针。 为了重置结构的值，作者使用了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在针对nullptr对其进行验证之前，已使用了'aValues'指针。 检查行：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诊断程序通常会检测到空指针取消引用的典型错误。 在这种情况下，我们有一个非常有趣的示例，值得特别注意。

从技术上讲，分析器是正确的，因为首先取消了对aValues指针的引用，然后再进行了检查，但是实际错误有所不同。 它是一个双指针，因此正确的代码应如下所示：

 *aValues = static_cast<nsILDAPBERValue **>( moz_xmalloc(numVals * sizeof(nsILDAPBERValue))); if (!*aValues) { ldap_value_free_len(values); return NS_ERROR_OUT_OF_MEMORY; } 

另一个类似的片段：

• V595在针对nullptr对其进行验证之前，已使用了'_retval'指针。 检查行：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); } .... } 

使用新的诊断程序可发现此错误，该诊断程序将在下一版分析仪中提供。 while循环条件中使用的所有变量都不会更改，因为变量ptr2和cSet在函数的主体中是混淆的。

网络

netwerk包含C接口和代码，用于低级访问网络（使用套接字以及文件和内存缓存）以及高级访问（使用各种协议，例如http，ftp，gopher，castanet）。 该代码也被称为“ netlib”和“ Necko”。

V501在'&&'运算符的左侧和右侧有相同的子表达式'connectStarted'。 nsSocketTransport2.cpp 1693

 nsresult nsSocketTransport::InitiateSocket() { .... if (gSocketTransportService->IsTelemetryEnabledAndNotSleepPhase() && connectStarted && connectCalled) { // <= good, line 1630 SendPRBlockingTelemetry( connectStarted, Telemetry::PRCONNECT_BLOCKING_TIME_NORMAL, Telemetry::PRCONNECT_BLOCKING_TIME_SHUTDOWN, Telemetry::PRCONNECT_BLOCKING_TIME_CONNECTIVITY_CHANGE, Telemetry::PRCONNECT_BLOCKING_TIME_LINK_CHANGE, Telemetry::PRCONNECT_BLOCKING_TIME_OFFLINE); } .... if (gSocketTransportService->IsTelemetryEnabledAndNotSleepPhase() && connectStarted && connectStarted) { // <= fail, line 1694 SendPRBlockingTelemetry( connectStarted, Telemetry::PRCONNECT_FAIL_BLOCKING_TIME_NORMAL, Telemetry::PRCONNECT_FAIL_BLOCKING_TIME_SHUTDOWN, Telemetry::PRCONNECT_FAIL_BLOCKING_TIME_CONNECTIVITY_CHANGE, Telemetry::PRCONNECT_FAIL_BLOCKING_TIME_LINK_CHANGE, Telemetry::PRCONNECT_FAIL_BLOCKING_TIME_OFFLINE); } .... } 

首先，我认为复制connectStarted变量只是冗余代码。 但是随后，我浏览了整个较长的函数，发现了一个类似的片段。 最有可能的是， connectCalled变量必须位于此处，而不是connectStarted变量。

V611使用“ new T []”运算符分配了内存，但使用“ delete”运算符释放了内存。 考虑检查此代码。 最好使用'delete [] mData;'。 检查行：233，222。DataChannel.cpp 233

 BufferedOutgoingMsg::BufferedOutgoingMsg(OutgoingMsg& msg) { size_t length = msg.GetLeft(); auto* tmp = new uint8_t[length]; // infallible malloc! memcpy(tmp, msg.GetData(), length); mLength = length; mData = tmp; mInfo = new sctp_sendv_spa; *mInfo = msg.GetInfo(); mPos = 0; } BufferedOutgoingMsg::~BufferedOutgoingMsg() { delete mInfo; delete mData; } 

mData指针指向一个数组，而不是单个对象。 由于缺少删除运算符的括号，导致在类析构函数中发生错误。

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

dom包含C接口和用于在Javascript中实现和跟踪DOM（文档对象模型）对象的代码。 它形成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”运算符释放了内存。 考虑检查“ key”变量后面的操作逻辑。 检查行：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函数后，必须使用free函数而不是delete运算符释放内存。

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”运行时库的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; } /* SocketPoll */ 

分析器检测到将数字常量分配给out_flags指针。 最有可能的是，人们忘记了取消引用它：

 if (fd->secret->alreadyConnected) { *out_flags = PR_POLL_WRITE; return PR_POLL_WRITE; } 

结论

还没有结束。 让新的代码评论成为！ Thunderbird和Firefox代码包含两个大型库：网络安全服务（NSS）和WebRTC（网络实时通信）。 我发现那里有一些令人信服的错误。 在这篇评论中，我将展示每个项目中的一个。

净水器

V597编译器可以删除“ 内存集 ”函数调用，该函数调用用于刷新“ 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密钥。 编译器将从代码中删除memset调用，因为在代码中的任何地方都不再使用newdeskey数组。

WebRTC

V519 '状态[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 。