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

转载 2012年03月23日 02:52:12

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

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

摘要

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

介绍

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

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

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

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

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

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

 数组和字符串处理的错误

数组和字符串处理的错误是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) /