Mozilla Thunderbird电子邮件客户端的“冒险”始于自动升级到68.0版。 此版本的显着功能是：在弹出通知中添加更多文本，默认情况下为深色主题。 我想尝试使用静态分析检测到一个错误。 这是使用PVS-Studio再次检查项目源代码的机会。 事实证明，到分析时为止，该错误已得到解决。 但是，由于我们提请注意该项目，因此我们可以写出其中发现的其他缺陷。
引言
新版本的Thunderbird的深色主题看起来非常漂亮。 我喜欢黑暗的主题。 已经在即时通讯工具，Windows，macOS中切换到它们。 很快，iPhone将升级到出现了黑暗主题的iOS 13。 为此，我什至不得不将iPhone 5S更改为较新的型号。 实际上，事实证明，深色主题需要开发人员花更多的精力来选择界面的颜色。 并非所有人都第一次解决这个问题。
因此，我在Thunderbird中的标准标签开始看起来像：
我使用六个标签（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) { .... } .... }
具有类似名称的变量的另一个奇怪的比较是
headers 。 与往常一样，有两种可能的解释：额外的支票或错字。
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)) {
通过按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) {
第一个条件和最后一个条件相同。 该代码表明该代码正在编写过程中。 可以肯定地说，在代码最终确定之后，错误会很明显地显现出来。 程序员可以更改注释的代码，但他永远都无法控制。 小心使用此代码。
V522可能会
取消引用空指针“行”。 第175章
NS_IMETHODIMP morkRowCellCursor::MakeCell(
在下面的行中可以取消引用
行空指针：
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, icalmime_local_action_map, get_string, data, 0 ); .... }
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) {
最初，我认为复制
connectStarted变量只是多余的代码，直到我查看了整个足够长的函数并找到了类似的片段。 最有可能的是，除了单个变量
connectStarted之外，这里还应该是一个变量
connectCalled 。
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];
mData指针指向一个数组，而不是单个对象。 他们在类析构函数中犯了一个错误，忘记为
delete运算符添加方括号。
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; } .... }
WinGI SHGetSpecialFolderPathW函数返回
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; }
分析器检测到将数字常量分配给
out_flags指针。 他们很可能忘记了取消引用他：
if (fd->secret->alreadyConnected) { *out_flags = PR_POLL_WRITE; return PR_POLL_WRITE; }
结论
这还没有结束。 是新的代码审查。 Thunderbird和Firefox代码包括两个主要的库：网络安全服务（NSS）和WebRTC（Web实时通信）。 有一些非常有趣的错误。 在这篇评论中，我将一次展示一个。
净水器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数组不再在此位置之外的代码中使用。
WebRTCV519 '状态[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更改为
state_low 。
这些项目中可能还有更多有趣的错误值得一提。 我们将在不久的将来做到这一点。 同时，在您的项目上尝试使用
PVS-Studio 。
如果您想与说英语的读者分享这篇文章，请使用以下链接：Svyatoslav Razmyslov。
雷鸟的黑暗主题是运行代码分析仪的原因 。