自2018年结束以来已经过去了三个月。 对于许多人来说,这只是飞逝,但是对于我们PVS-Studio开发人员而言,这是相当重要的一年。 我们正在竭尽全力,无所畏惧地竞相传播有关静态分析的信息,并在寻找使用C,C ++,C#和Java语言编写的开源项目中的错误。 在本文中,我们为您收集了其中最有趣的前十名!
为了找到最有趣的地方,我们使用了
PVS-Studio静态代码分析器。 它可以检测用上面列出的语言编写的代码中的错误和潜在漏洞。
如果您对自己寻找错误感到很兴奋,欢迎随时下载并尝试使用我们的分析仪。 我们为学生和热情的开发人员提供了
免费的分析器版本,为开源项目的开发人员提供了
免费的许可证 ,还为全世界和他的狗提供了
试用版 。 谁知道,也许到明年您将能够创建自己的前十名? :)
注意:我邀请您进行检查,并在查看分析仪警告之前,尝试自己发现缺陷。 您将发现多少个错误?
第十名
来源:
再次进入太空:独角兽如何拜访恒星在检查名为Stellarium的虚拟天象仪时检测到此错误。
上面的代码片段虽然很小,但充满了一个非常棘手的错误:
Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3) : distance(0.0f), sDistance(0.0f) { Plane(v1, v2, v3, SPolygon::CCW); }
找到了吗?
PVS-Studio警告 :
V603已创建对象,但未使用该对象。 如果要调用构造函数,则应使用“ this-> Plane :: Plane(....)”。 平面cpp 29
代码作者打算使用嵌套在主对象中的另一个构造函数来初始化某些对象的字段。 嗯,与其相反,他只是设法创建了一个在离开其作用域时销毁的临时对象。 这样,几个对象的字段将保持未初始化。
作者应该使用在C ++ 11中引入的委托构造函数,而不是嵌套的构造函数调用。 例如,他本可以这样写:
Plane::Plane(Vec3f& v1, Vec3f& v2, Vec3f& v3) : Plane(v1, v2, v3, SPolygon::CCW) { distance = 0.0f; sDistance = 0.0f; }
这样,所有必要的字段都将被正确初始化。 这不是很好吗?
第九名
来源:
Perl 5:如何在宏中隐藏错误第九位是一个非常出色的宏,它的所有美都引人注目。
在收集撰写文章的错误时,我的同事Svyatoslav遇到了由分析器发出的与宏用法有关的警告。 这是:
PP(pp_match) { .... MgBYTEPOS_set(mg, TARG, truebase, RXp_OFFS(prog)[0].end); .... }
为了找出问题所在,Svyatoslav进行了更深入的挖掘。 他打开了宏定义,发现其中包含多个嵌套宏,其中一些反过来又具有嵌套宏。 很难理解这一点,因此他必须使用预处理的文件。 可悲的是,它没有帮助。 这是Svyatoslav在上一行代码中找到的:
(((targ)->sv_flags & 0x00000400) && (!((targ)->sv_flags & 0x00200000) || S_sv_only_taint_gmagic(targ)) ? (mg)->mg_len = ((prog->offs)[0].end), (mg)->mg_flags |= 0x40 : ((mg)->mg_len = (((targ)->sv_flags & 0x20000000) && !__builtin_expect(((((PL_curcop)->cop_hints + 0) & 0x00000008) ? (_Bool)1 :(_Bool)0),(0))) ? (ssize_t)Perl_utf8_length( (U8 *)(truebase), (U8 *)(truebase)+((prog->offs)[0].end)) : (ssize_t)((prog->offs)[0].end), (mg)->mg_flags &= ~0x40));
PVS-Studio警告 :
V502也许“?:”操作员的工作方式与预期的不同。 '?:'运算符的优先级低于'&&'运算符。 pp_hot.c 3036
我认为,仅注意到这样的错误将具有挑战性。 我们一直在研究此代码很长一段时间,但是坦率地说,我们没有发现任何错误。 无论如何,这是一个可读性很差的有趣示例。
他们说宏是邪恶的。 当然,在某些情况下,宏是必不可少的,但是如果您可以将宏替换为函数,则一定要这样做。
嵌套宏尤其充满陷阱。 不仅因为很难理解它们,还因为它们会产生不可预测的结果。 如果程序员在这样的宏中犯了一个错误-在宏中找到它比在函数中发现要困难得多。
第八名
资料来源:
铬:其他错误下一个示例来自有关Chromium项目分析的系列文章。 该错误隐藏在WebRTC库中。
std::vector<SdpVideoFormat> StereoDecoderFactory::GetSupportedFormats() const { std::vector<SdpVideoFormat> formats = ....; for (const auto& format : formats) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } } return formats; }
PVS-Studio警告:在基于范围的for循环中使用的“格式”容器的
V789 CWE-672迭代器在调用“ push_back”函数时变得无效。 stereocodecfactory.cc 89
错误是
格式向量的大小在基于范围的for循环内变化。 基于范围的循环基于迭代器,这就是为什么在此类循环中更改容器大小可能导致这些迭代器无效的原因。
如果使用显式使用迭代器重写循环,则该错误仍然存在。 为了清楚起见,我可以引用以下代码:
for (auto format = begin(formats), __end = end(formats); format != __end; ++format) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } }
例如,当使用
push_back方法时,可能会发生向量重新分配-这样,迭代器将寻址无效的内存位置。
为避免此类错误,请遵循以下规则:切勿在绑定了此容器条件的循环内更改容器大小。 它还涉及基于范围的循环和使用迭代器的循环。 欢迎您阅读有关StackOverflow的
讨论 ,该
讨论涵盖导致迭代器无效的操作主题。
第七名
来源:
Godot:关于静态分析仪的常规使用来自游戏行业的第一个示例将是我们在Godot游戏引擎中找到的代码段。 可能需要一些工作才能注意到该错误,但是我敢肯定,我们精通的读者将对此进行应对。
void AnimationNodeBlendSpace1D::add_blend_point( const Ref<AnimationRootNode> &p_node, float p_position, int p_at_index) { ERR_FAIL_COND(blend_points_used >= MAX_BLEND_POINTS); ERR_FAIL_COND(p_node.is_null()); ERR_FAIL_COND(p_at_index < -1 || p_at_index > blend_points_used); if (p_at_index == -1 || p_at_index == blend_points_used) { p_at_index = blend_points_used; } else { for (int i = blend_points_used - 1; i > p_at_index; i++) { blend_points[i] = blend_points[i - 1]; } } .... }
PVS-Studio警告: V621 CWE-835考虑检查“ for”操作员。 循环有可能执行不正确或根本不执行。 animation_blend_space_1d.cpp 113
让我们仔细看看循环条件。 计数器变量由
blend_points_used-1值初始化。 此外,从之前的两次检查(在
ERR_FAIL_COND和
if中 )
来看 ,很明显,到
blend_points_used循环执行时,
blend_points_used将始终大于
p_at_index 。 因此,要么循环条件始终为真,要么根本不执行循环。
如果
blend_points_used-1 == p_at_index ,则循环不会执行。
在所有其他情况下,检查
i> p_at_index始终为true,因为每次循环迭代时
i计数器都会增加。
循环似乎是永恒的,但事实并非如此。
首先,将发生
i变量的整数溢出(这是未定义的行为)。 这意味着,我们不应该依赖它。
如果
i是
unsigned int ,则在计数器达到最大可能值之后,运算符
i ++会将其变为
0 。 这种行为由标准定义,称为“无符号包装”。 但是,您应该意识到使用这种机制
也不是一个好主意 。
这是第一点,但我们还有第二点! 情况是,我们甚至都不会达到整数溢出。 数组索引将更早地超出范围。 这意味着将尝试访问为该阵列分配的块之外的内存。 这也是未定义的行为。 一个经典的例子:)
我可以给您一些建议,以更容易避免类似的错误:
- 编写简单易懂的代码
- 彻底检查代码并为新编写的代码编写更多测试
- 使用静态分析仪;)
第六名
资料来源:
亚马逊伐木场:痛苦的尖叫这是游戏开发行业的另一个示例,即Amazon Lumberyard AAA引擎的源代码。
void TranslateVariableNameByOperandType(....) {
PVS-Studio警告 :
V523'then '语句等效于'else'语句。 toglsloperand.c 700
Amazon Lumberyard被开发为跨平台引擎。 因此,开发人员尝试支持尽可能多的编译器。 从评论中可以看出,程序员Igor反对Qualcomm编译器。
我们不知道他是否能够执行任务并通过“偏执”编译器检查涉水,但是他留下了非常奇怪的代码。 奇怪的是,
if语句的
then-和
else-分支都包含绝对相同的代码。 这种错误很可能是由于使用草率的复制粘贴方法导致的。
我什至不知道该怎么建议。 因此,我只希望Amazon Lumberyard开发人员在修复错误方面一切顺利,并为开发人员Igor带来好运!
第五名
资料来源:
再次证明,PVS-Studio分析仪比人更专心下一个示例发生了一个有趣的故事。 我的同事Andrey Karpov正在准备有关Qt框架的另一项检查的文章。 在写出一些明显的错误时,他偶然发现了分析仪警告,认为这是错误的。 这是该代码片段及其警告:
QWindowsCursor::CursorState QWindowsCursor::cursorState() { enum { cursorShowing = 0x1, cursorSuppressed = 0x2 }; CURSORINFO cursorInfo; cursorInfo.cbSize = sizeof(CURSORINFO); if (GetCursorInfo(&cursorInfo)) { if (cursorInfo.flags & CursorShowing)
PVS-Studio警告: V616 CWE-480按位操作中使用名为“ CursorShowing”的常量,其值为0。 qwindowscursor.cpp 669
这意味着,PVS-Studio在该地方抱怨,这显然没有错误!
CursorShowing常量不可能为
0 ,因为它上面的几行由
1初始化。
由于Andrey使用的分析仪版本不稳定,因此他对警告的正确性提出了质疑。 他仔细查看了这段代码,但仍然没有发现错误。 最终,他在bugtracker中给了它一个误报,以便其他同事可以纠正这种情况。
只有详细的分析表明,PVS-Studio再次比一个人更小心。 将
0x1值分配给名为
cursorShowing的命名常量,而
CursorShowing参与按位“与”运算。 这是两个完全不同的常数,第一个以小写字母开头,第二个以大写字母开头。
代码成功编译,因为类
QWindowsCursor确实包含具有此名称的常量。 这是它的定义:
class QWindowsCursor : public QPlatformCursor { public: enum CursorState { CursorShowing, CursorHidden, CursorSuppressed }; .... }
如果您没有为枚举常量明确分配值,则默认情况下将对其进行初始化。 由于
CursorShowing是枚举中的第一个元素,因此将其分配为
0 。
为避免此类错误,您不应给实体命名太相似的名称。 如果实体属于同一类型或可以彼此隐式转换,则应特别遵循此规则。 在这种情况下,几乎不可能注意到该错误,但是不正确的代码仍将被编译并存在于项目内部。
第四名
资料来源:
处理输入数据时,请小心翼翼我们越来越接近前三名决赛选手,而下一个是FreeSWITCH项目的错误。
static const char *basic_gets(int *cnt) { .... int c = getchar(); if (c < 0) { if (fgets(command_buf, sizeof(command_buf) - 1, stdin) != command_buf) { break; } command_buf[strlen(command_buf)-1] = '\0'; break; } .... }
PVS-Studio警告: V1010 CWE-20索引:“ strlen(command_buf)”中使用了未经检查的污染数据。
分析器警告您在
strlen(command_buf)-1表达式中使用了一些未经检查的数据。 确实:如果用C语言来讲
command_buf是一个空字符串(仅包含字符'\ 0'),则
strlen(command_buf)将返回
0 。 在这种情况下,将访问
command_buf [-1] ,这是未定义的行为。 不好
对该错误的实际热情不是
为什么发生,而是
如何发生。 此错误是您自己“触摸”的最出色的示例之一。 您可以运行FreeSwitch,执行一些操作,这些操作将导致上述代码片段的执行,并将空字符串传递给程序的输入。
结果,随着手的微妙运动,工作程序变成无效! 您可以通过上述链接在源文章中找到有关如何重现此错误的详细信息。 同时,让我为您提供一个令人欣慰的结果:
请记住,输出数据可以是任何数据,因此您应该始终检查它。 这样,分析仪将不会抱怨,程序将变得更加可靠。
现在是时候为我们的获胜者而战:我们现在已经进入残局了! 顺便说一句,臭虫决赛选手已经等了很长时间,然后感到无聊,甚至开始变得笨拙。 看看他们在我们外出时的演出吧!
第三名
资料来源:
NCBI基因组工作台:受威胁的科学研究来自NCBI Genome Workbench项目的代码段(这是用于研究和分析基因数据的一组工具)使前3名获奖者获奖。 即使您不必是转基因的超人也可以找到此bug,但很少有人知道在这里犯错误的可能性。
void tds_answer_challenge(....) { .... if (ntlm_v == 1) { .... memset(hash, 0, sizeof(hash)); memset(passwd_buf, 0, sizeof(passwd_buf)); ... } else { .... } }
PVS-Studio警告:- V597编译器可以删除“ 内存集 ”函数调用,该函数调用用于刷新“哈希”缓冲区。 memset_s()函数应用于擦除私有数据。 Challenge.c 365
- V597编译器可以删除“ 内存集 ”函数调用,该函数调用用于刷新“ passwd_buf”缓冲区。 memset_s()函数应用于擦除私有数据。 第366章
您找到错误了吗? 如果是,则您是一个attaboy!..或转基因的超人。
事实是,现代的优化编译器能够做很多事情,以使构建的程序更快地运行。 包括编译器现在可以跟踪传递给
memset的缓冲区的事实。
在这种情况下,他们可以删除对
memset的“不必要的”调用,并拥有所有权利。 然后,存储重要数据的缓冲区可能会保留在内存中,使攻击者感到高兴。
在这种背景下,这种极客评论“安全最好是书呆子”听起来更有趣。 从针对该项目的少量警告来看,其开发人员会尽力做到精确并编写安全的代码。 但是,正如我们所看到的,人们可以轻易地忽略这种安全缺陷。 根据常见弱点枚举,此缺陷归类为
CWE-14 :清除代码以清除缓冲区的编译器。
您应该使用
memset_s()函数,以便安全地进行内存释放。 该函数比
memset()更安全,并且不能被编译器忽略。
第二名
资料来源:
PVS-Studio被证明比三个半程序员更具吸引力我们的一位客户将银牌得主送给了我们。 他确信分析仪会发出一些误报。
叶夫根尼(Evgeniy)收到了这封电子邮件,仔细检查后将其发送给Svyatoslav。 Svyatoslav仔细查看了客户发送的那段代码,心想:“分析仪怎么可能犯了这样的错误?”。 于是他去找安德烈咨询。 他还检查了该位置并确定:实际上,分析仪产生了误报。
因此,需要修复。 仅在Svyatoslav开始制作综合示例以在我们的错误跟踪器中创建任务之后,他才发现了问题所在。
没有一个程序员可以找到这些错误,但是它们确实在代码中。 坦白地说,尽管分析仪清楚地为错误的地方发出警告,但本文的作者也没有找到它们!
您会发现这种狡猾的虫子吗? 测试自己的警惕性和专心程度。
PVS-Studio警告:- V560条件表达式的一部分始终为false:(ch> = 0x0FF21)。 解码w.cpp 525
- V560条件表达式的一部分始终为true:(ch <= 0x0FF3A)。 解码w.cpp 525
- V560条件表达式的一部分始终为false:(ch> = 0x0FF41)。 解码w.cpp 525
- V560条件表达式的一部分始终为true:(ch <= 0x0FF5A)。 解码w.cpp 525
如果您做到了-恭喜您!
错误在于以下事实:逻辑否定运算符(!)不适用于整个条件,而仅适用于其第一个子表达式:
!((ch >= 0x0FF10) && (ch <= 0x0FF19))
如果满足此条件,则
ch变量值将位于[0x0FF10 ... 0x0FF19]范围内。 因此,四个进一步的比较已经毫无意义:它们将始终是对还是错。
为避免此类错误,值得遵循一些规则。 首先,将代码像表一样对齐是非常方便和有益的。 其次,您不应该在表达式上加上括号。 例如,此代码可以这样重写:
const bool isLetterOrDigit = (ch >= 0x0FF10 && ch <= 0x0FF19)
这样,括号将更少,而另一方面-您更有可能注意到偶然的错误。
樱桃就在这里-让我们继续前进吧!
第一名
来源:
Shocked System:传奇系统Shock的源代码中有趣的错误今天的决赛入围者是传奇的System Shock的错误! 它是1994年发行的很久以前的游戏,它成为诸如Dead Space,BioShock和Deus Ex等标志性游戏的前身和灵感。
但是首先,我要承认一些事情。 我现在要向您展示的内容不包含任何错误。 实际上,它甚至都不是一段代码,但是我忍不住与您共享它!
问题是,在分析游戏的源代码时,我的同事Victoria发现了很多有趣的评论。 在不同的片段中,她发现了一些机智和讽刺的话,甚至是诗歌。
这就是最近90年代开发人员在游戏中留下的评论的样子……顺便说一下,System Shock的首席设计师Doug Church也在忙于编写代码。 谁知道,也许其中一些评论是他写的? 希望,毛巾上的东西不是他的手工:)
结论
最后,我要感谢
我的同事寻找新的bug并在文章中进行介绍。 谢谢你们! 没有你,这篇文章不会那么有趣。
另外,我想介绍一下我们的成就,因为我们整年都在忙于寻找错误。 我们还一直在开发和改进分析仪,从而对其进行了重大更改。
例如,我们增加了对几个新编译器的支持,并扩展了诊断规则的列表。 我们还实施了对
MISRA C和MISRA C ++标准的初步支持。 最重要且耗时的新功能是对新语言的支持。 是的,我们现在可以分析
Java代码! 而且,我们还有一个
新图标 :)
我还要感谢我们的读者。 感谢您阅读我们的文章并写信给我们! 您反应灵敏,对我们如此重要!
我们2018年的十大C ++错误已经结束。 您最喜欢什么片段,为什么? 您在2018年遇到过一些有趣的例子吗?
祝一切顺利,下次见!