100个开源C/C++项目中的bugs（一）数组和字符串处理的错误

from:http://www.oschina.net/question/1579_45444

100个开源C/C++项目中的bugs

摘要
介绍
发现错误样本的各类开源项目
- 数组和字符串处理的错误
- 未定义行为
- 与运算符优先级相关的错误
- 格式化输出错误
- 代码打印错误例子
- 不正确的使用基本函数和类
- 无效代码的例子
- 总为 true 或 false 的条件
- 代码漏洞
- 复制粘贴
- 逾期空指针检查
- 杂项

摘要

本文演示静态代码分析的能力. 提供了100个已在开源C/C++项目中发现的错误例子给读者研究。所有的错误已经被PVS-Studio静态代码分析工具发现。

介绍

我们不想让你阅读文档感到厌倦,并马上给你传达错误例子.

那些想了解什么是静态代码分析的读者，请跟随链接。想了解什么是PVS-Studio的读者，可下载试用版.请看该页：http://www.viva64.com/en/pvs-studio/.

当然，还有一件事，请查看我们的帖子: "FAQ for those who have read our articles"

发现错误样本的各个开源项目

错误实例將被归为几类。这种分法是很相对性的。同一个错误常常同时属于打印错误和不正确的数组操作错误.

当然我们仅从每个项目中列出少数几个错误。如果我们描述所以已发现的错误，那將要编写一本指南书了。以下是被分析的项目：

Apache HTTP Server - http://httpd.apache.org/
Audacity - http://audacity.sourceforge.net/
Chromium - http://www.chromium.org/
Clang - http://clang-analyzer.llvm.org/
CMake - http://www.cmake.org/
Crystal Space 3D SDK - http://www.crystalspace3d.org/main/Main_Page
Emule - http://www.emule.com/
FAR Manager - http://www.farmanager.com/
FCE Ultra - http://fceux.com/web/home.html
Fennec Media Project - http://fennec.sourceforge.net/
G3D Content Pak - http://sourceforge.net/projects/g3d-cpp/
IPP Samples - http://www.viva64.com/go.php?url=449
Lugaru - http://www.wolfire.com/lugaru
Miranda IM - http://www.miranda-im.org/
MySQL - http://www.mysql.com/
Newton Game Dynamics - http://newtondynamics.com/forum/newton.php
Notepad++ - http://notepad-plus-plus.org/
Pixie - http://www.renderpixie.com/
PNG library - http://libpng.org/pub/png/
QT - http://qt.nokia.com/products/
ReactOS - http://www.reactos.org/en/
Shareaza - http://www.shareaza.com/
SMTP Client with SSL/TLS - http://www.codeproject.com/KB/IP/smtp_ssl.aspx
StrongDC++ - http://strongdc.sourceforge.net/index.php?lang=eng
Swiss-Army Knife of Trace - http://www.codeproject.com/KB/trace/tracetool.aspx
TortoiseSVN - http://tortoisesvn.net/
Ultimate TCP/IP - http://www.codeproject.com/KB/MFC/UltimateTCPIP.aspx
VirtualDub - http://www.virtualdub.org/
WinDjView - http://windjview.sourceforge.net/
WinMerge - http://winmerge.org/
Wolfenstein 3D - http://en.wikipedia.org/wiki/Wolfenstein_3D
Crypto++ - http://www.cryptopp.com/
Quake-III-Arena - https://github.com/id-Software/Quake-III-Arena
And some others.

数组和字符串处理的错误

数组和字符串处理的错误是C/C++程序中最大的一类。它以程序员可以高效处理低级内存问题为代价. 本文中，我们將展示一小部分被PVS-Studio分析器发现的此类错误。但我们相信任何C/C++程序员都了解他们有多巨大和阴险.

例 1. Wolfenstein 3D项目。对象仅被部分清除：

`1`	`void` `CG_RegisterItemVisuals(` `int` `itemNum ) {`

2 ...

`3`	`itemInfo_t *itemInfo;`

4 ...

`5`	`memset( itemInfo, 0,` `sizeof( &itemInfo ) );`

6 ...

7 }

The error was found through the V568 diagnostic: It's odd that the argument of sizeof() operator is the '&itemInfo' expression. cgame cg_weapons.c 1467.

sizeof() 操作符计算的是指针大小而非‘itemInfo_t’结构体大小。必须写成"sizeof(*itemInfo)"。

例 2. Wolfenstein 3D项目。矩阵仅被部分清除：

`1`	`ID_INLINE mat3_t::mat3_t(` `float` `src[ 3 ][ 3 ] ) {`

`2`	`memcpy( mat, src,` `sizeof( src ) );`

3 }

The error was found through the V568 diagnostic: sizeof()操作返回的是指针的大小，非数组的大小，在'sizeof(src)'表达式中. Splines math_matrix.h 94.

通常开发者期望 'sizeof(src)' 操作返回"3*3*sizeof(float)"字节的数组大小。但根据语言规范，'src'仅是一个指针，而不是一个数组。因此，该矩阵只被部分的复制。‘'memcpy'’函数將拷贝4或8字节（指针大小），这依赖于代码是32位还是64位的.

如果你希望整个矩阵都被拷贝，你可以给函数传递该数组的引用。以下是正确的代码：

`1`	`ID_INLINE mat3_t::mat3_t(` `float` `(&src)[3][3] )`

2 {

`3`	`memcpy( mat, src,` `sizeof( src ) );`

4 }

例 3. FAR Manage 项目。数组仅被部分清除：

The error was found through the V579: memset 函数接收指针和它的大小作为参数. 这可能会引发错误。检测第三个参数。far treelist.hpp 66.

最有可能的是, 计算待清除的元素数量的乘法操作丢失了, 而该代码须如下所示:

"memset(Last, 0, LastCount * sizeof(*Last));".

例 4. ReactOS. 不正确的字符串长度计算.

`01`	`static` `const` `PCHAR` `Nv11Board =` `"NV11 (GeForce2) Board";`

`02`	`static` `const` `PCHAR` `Nv11Chip =` `"Chip Rev B2";`

`03`	`static` `const` `PCHAR` `Nv11Vendor =` `"NVidia Corporation";`

04

05 BOOLEAN

`06`	`IsVesaBiosOk(...)`

07 {

08 ...

`09`	`if` `(!(strncmp(Vendor, Nv11Vendor,` `sizeof(Nv11Vendor))) &&`

`10`	`!(strncmp(Product, Nv11Board,` `sizeof(Nv11Board))) &&`

`11`	`!(strncmp(Revision, Nv11Chip,` `sizeof(Nv11Chip))) &&`

`12`	`(OemRevision == 0x311))`

13 ...

14 }

该错误经由V579诊断：strncmp 函数接收了指针和它的大小做为参数。这可能是个错误。查看第三个参数。vga vbe.c 57

'strncmp'的函数的调用仅仅比较了前几个字符，而不是整个字符串。这里的错误是：sizeof()操作，在这种情况下用来计算字符串长度绝对不适宜。sizeof()操作实际上只计算了指针的大小而不是string的字节数量。

关于该错误最讨厌和阴险的是，该代码大多数时候都如预期的工作。99％的情况下，比较前几个字符就足够了。但剩下的1%能带给你愉快和长时间的调试过程。

例 5. VirtualDub 项目. 数据越界（明确的下标).

`01`	`struct` `ConvoluteFilterData {`

`02`	`long` `m[9];`

`03`	`long` `bias;`

`04`	`void` `*dyna_func;`

`05`	`DWORD` `dyna_size;`

`06`	`DWORD` `dyna_old_protect;`

`07`	`BOOL` `fClip;`

08 };

09

`10`	`static` `unsigned` `long` `__fastcall do_conv(`

`11`	`unsigned` `long` `*data,`

`12`	`const` `ConvoluteFilterData *cfd,`

`13`	`long` `sflags,` `long` `pit)`

14 {

`15`	`long` `rt0=cfd->m[9], gt0=cfd->m[9], bt0=cfd->m[9];`

16 ...

17 }

The code was found through the V557 diagnostic: 数组可能越界。下标‘9’已经指向数组边界外. VirtualDub f_convolute.cpp 73

这不是一个真正的错误，但是个好的诊断。解释： http://www.viva64.com/go.php?url=756.

例 6. CPU Identifying Tool 项目. 数组越界（宏中的下标）

`01`	`#define FINDBUFFLEN 64 // Max buffer find/replace size`

02 ...

`03`	`int` `WINAPI Sticky (...)`

04 {

05 ...

`06`	`static` `char` `findWhat[FINDBUFFLEN] = {'\0'};`

07 ...

`08`	`findWhat[FINDBUFFLEN] =` `'\0';`

09 ...

10 }

The error was found through the V557 diagnostic:数组可能越界.下标‘64’已经指向数组边界外。stickies stickies.cpp 7947

该错误与上例类似。末端 null 已被写到数组外。正确代码是："findWhat[FINDBUFFLEN - 1] = '\0';".

例 7. Wolfenstein 3D 项目. 数组越界(不正确的表达式).

`01`	`typedef` `struct` `bot_state_s`

02 {

03 ...

`04`	`char` `teamleader[32];` `//netname of the team leader`

05 ...

`06`	`} bot_state_t;`

07

`08`	`void` `BotTeamAI( bot_state_t *bs ) {`

09 ...

`10`	`bs->teamleader[sizeof( bs->teamleader )] =` `'\0';`

11 ...

12 }

The error was found through the V557 diagnostic: 数组可能越界. 'sizeof (bs->teamleader)' 下标已指向数组边界外。game ai_team.c 548

这是又一个数组越界的例子, 当使用了明确声明的下标时候. 这些例子说明了,这些不起眼的错误比看上去更广泛.

末端的 null 被写到了 'teamleader' 数组的外部。下面是正确代码：

`1`	`bs->teamleader[`

`2`	`sizeof(bs->teamleader) /` `sizeof(bs->teamleader[0]) - 1`

`3`	`] =` `'\0';`

Example 8. Miranda IM 项目. 仅字符串的部分被拷贝.

`01`	`typedef` `struct` `_textrangew`

02 {

`03`	`CHARRANGE chrg;`

`04`	`LPWSTR` `lpstrText;`

`05`	`} TEXTRANGEW;`

06

`07`	`const` `wchar_t* Utils::extractURLFromRichEdit(...)`

08 {

09 ...

`10`	`::CopyMemory(tr.lpstrText, L"mailto:", 7);`

11 ...

12 }

The error was found through the V512 diagnostic: 一个 'memcpy' 函数的调用將导致缓冲区上溢或下溢。tabsrmm utils.cpp 1080

如果使用Unicode字符集，一个字符占用2或4字节（依赖于编译器使用的数据模型）而不是一字节。不幸的是，程序员们很容易忘记，你常常会在程序中看到类似该例子的污点。

'CopyMemory' 函数將只拷贝L"mailto:"字符串的部分，因为他处理字节，而不是字符。你可以使用一个更恰当的字符串拷贝函数修复该代码，或者,至少是，将 sizeof(wchar_t) 与数字7 相乘.

例 9. CMake 项目. 循环中的数组越界.

`01`	`static` `const` `struct` `{`

`02`	`DWORD` `winerr;`

`03`	`int` `doserr;`

`04`	`} doserrors[] =`

05 {

06 ...

07 };

08

`09`	`static` `void`

`10`	`la_dosmaperr(unsigned` `long` `e)`

11 {

12 ...

`13`	`for` `(i = 0; i <` `sizeof(doserrors); i++)`

14 {

`15`	`if` `(doserrors[i].winerr == e)`

16 {

`17`	`errno` `= doserrors[i].doserr;`

18 return;

19 }

20 }

21 ...

22 }

The error was found through the V557 diagnostic: 数组可能越界.'i'下标值可到达 367. cmlibarchive archive_windows.c 1140, 1142

该错误处理本身就包含了错误。sizeof() 操作符以字节数返回数组大小而不是里面的元素的数量。因此,该程序將在循环中试图搜索多于本应搜索的元素.下面是正确的循环：

`1`	`for` `(i = 0; i <` `sizeof(doserrors) /` `sizeof(*doserrors); i++)`

例 10. CPU Identifying Tool 项目. A string is printed into itself.

`01`	`char` `* OSDetection ()`

02 {

03 ...

`04`	`sprintf(szOperatingSystem,`

`05`	`"%sversion %d.%d %s (Build %d)",`

`06`	`szOperatingSystem,`

`07`	`osvi.dwMajorVersion,`

`08`	`osvi.dwMinorVersion,`

`09`	`osvi.szCSDVersion,`

`10`	`osvi.dwBuildNumber & 0xFFFF);`

11 ...

`12`	`sprintf` `(szOperatingSystem,` `"%s%s(Build %d)",`

`13`	`szOperatingSystem, osvi.szCSDVersion,`

`14`	`osvi.dwBuildNumber & 0xFFFF);`

15 ...

16 }

This error was found through the V541 diagnostic: It is dangerous to print the string 'szOperatingSystem' into itself. stickies camel.cpp 572, 603

一个格式化字符串打印到自身的企图，可能会导致不良后果。

代码的执行结果依赖于输入数据，结果未知。可能的，结果会是一个无意义字符串或一个 Access Violation 將发生。

该错误可以归为 "脆弱代码" 一类.在某些程序中，向代码提供特殊字符,你可以利用这些代码片段触发缓冲区溢出或者被入侵者利用.

例 11. FCE Ultra 项目。某字符串未获得足够内存

`1`	`int` `FCEUI_SetCheat(...)`

2 {

3 ...

`4`	`if((t=(char` `*)realloc(next->name,strlen(name+1))))`

5 ...

6 }

The error was found through the V518 diagnostic: 'realloc' 函数通过 'strlen(expr)' 申请了数量奇怪的内存. 可能正确的变体是 'strlen(expr) + 1'. fceux cheat.cpp 609

该错误由一个错误打印导致。strlen() 函数的参数必须是 "name" 指针而不是 "name+1" 指针。因此，realloc 函数比实际需要少申请了两个字节：1个字节丢失了，因为1没有被加入到字符串长度中。另一个字节也丢失了，因为'strlen'函数计算长度时跳过了第一个字符。

例 12. Notepad++ 项目。部分的数组清理。

`1`	`#define CONT_MAP_MAX 50`

`2`	`int` `_iContMap[CONT_MAP_MAX];`

3 ...

`4`	`DockingManager::DockingManager()`

5 {

6 ...

`7`	`memset(_iContMap, -1, CONT_MAP_MAX);`

8 ...

9 }

The error was found through the V512 diagnostic:memset 函数的调用將导致缓冲区上溢或下溢. notepadPlus DockingManager.cpp 60

这是又一个数组元素数量与数组大小混淆的例子。一个通过 sizeof(int) 的乘法操作丢失了。

我们可以继续给你指出更多我们在各种项目中发现的数组处理错误例子。但我们不得不停止了。