C/C++100个典型的Bugs(三)
ff, keys,&dwReturnedValue,0);
…
}
(lParam >> 16) && 0xff没有什么意义,求值结果总是true。这里的代码应该是(lParam >> 16) & 0xff。
[#12] Fennec Media Project项目 – 额外的分号
int settings_default(void)
{
…
for(i=0; i<16; i++);
for(j=0; j<32; j++)
{
settings.conversion.equalizer_bands.boost[i][j] = 0.0;
settings.conversion.equalizer_bands.preamp[i] = 0.0;
}
}
这又是一个实际逻辑与代码缩进不符的例子。作者的原意是这样的:
for(i=0; i<16; i++)
{
for(j=0; j<32; j++)
{
settings.conversion.equalizer_bands.boost[i][j] = 0.0;
settings.conversion.equalizer_bands.preamp[i] = 0.0;
}
}
但实际执行代码逻辑却是:
for(i=0; i<16; i++)
{
;
}
for(j=0; j<32; j++)
{
settings.conversion.equalizer_bands.boost[i][j] = 0.0;
settings.conversion.equalizer_bands.preamp[i] = 0.0;
}
这一切都是那个;导致的。
六、对基本函数和类的误用
[#2] TortoiseSVN项目 – remove函数的误用
STDMETHODIMP CShellExt::Initialize(….)
{
…
ignoredprops = UTF8ToWide(st.c_str());
// remove all escape chars ('\\')
std::remove(ignoredprops.begin(), ignoredprops.end(), '\\');
break;
…
}
作者意图删除所有'\\',但他用错了函数,remove函数只是交换元素的位置,将要删除的元素交换到尾部trash,并且返回指向trash首地址的iterator。正确的做法应该是"v.erase(remove(v.begin(), v.end(), 2), v.end())"。
[#5] Pixie项目 – 在循环中使用alloca函数
inline void triangulatePolygon(…) {
…
for (i=1;i
…
do {
…
do {
…
CTriVertex *snVertex =
(CTriVertex *)alloca(2*sizeof(CTriVertex));
…
} while(dVertex != loops[0]);
…
} while(sVertex != loops[i]);
…
}
…
}
alloca函数在栈上分配内存,因此在循环中使用alloca可能会很快导致栈溢出。
七、无意义的代码
[#1] IPP Samples项目 – 不完整的条件
void lNormalizeVector_32f_P3IM(Ipp32f *vec[3],
Ipp32s* mask, Ipp32s len)
{
Ipp32s i;
Ipp32f norm;
for(i=0; i
if(mask<0) continue;
norm = 1.0f/sqrt(vec[0][i]*vec[0][i]+
vec[1][i]*vec[1][i]+vec[2][i]*vec[2][i]);
vec[0][i] *= norm; vec[1][i] *= norm; vec[2][i] *= norm;
}
}
mask是Ipp32s类型指针,这样if (mask< 0)这句代码显然没啥意义,正确的代码应该是:
if (mask[i] < 0) continue;
[#2] QT项目 – 重复的检查
Q3TextCustomItem* Q3TextDocument::parseTable(…)
{
…
while (end < length
&& !hasPrefix(doc, length, end, QLatin1String("
&& !hasPrefix(doc, length, end, QLatin1String("
&& !hasPrefix(doc, length, end, QLatin1String("
&& !hasPrefix(doc, length, end, QLatin1String("
&& !hasPrefix(doc, length, end, QLatin1String("
&& !hasPrefix(doc, length, end, QLatin1String("
&& !hasPrefix(doc, length, end, QLatin1String("
&& !hasPrefix(doc, length, end, QLatin1String("
…
}
这里对"
八、总是True或False的条件
[#1] Shareaza项目 – char类型的值范围
void CRemote::Output(LPCTSTR pszName)
{
…
CHAR* pBytes = new CHAR[ nBytes ];
hFile.Read( pBytes, nBytes );
…
if ( nBytes > 3 && pBytes[0] == 0xEF &&
pBytes[1] == 0xBB && pBytes[2] == 0xBF )
{
pBytes += 3;
nBytes -= 3;
bBOM = true;
}
…
}
表达式"pBytes[0] == 0xEF"总是False。char类型的值范围是-128~127 < 0xEF,因此这个表达式总是False,导致整个if condition总是为False,与预期逻辑不符。
[#3] VirtualDub项目 – 无符号类型总是>=0
typedef unsigned short wint_t;
…
void lexungetc(wint_t c) {
if (c < 0)
return;
g_backstack.push_back(c);
}
c是unsigned short类型,永远不会小于0,也就是说if (c < 0)永远为False。
[#8] MySQL项目 – 条件错误
enum enum_mysql_timestamp_type
str_to_datetime(…)
{
…
else if (str[0] != ‘a’ || str[0] != 'A')
continue; /* Not AM/PM */
…
}
if (str[0] != ‘a’ || str[0] != 'A')这个条件永远为真。也许这块本意是想用&&。
九、代码
漏洞
导致漏洞的代码错误实际上也都是笔误、不正确的条件以及不正确的数组操作等。但这里还是想将一些特定错误划归为一类,因为入侵者可以利用这些错误来攻击你的代码,获取其利益。
[#1] Ultimate TCP/IP项目 – 空字符串的错误检查
char *CUT_CramMd5::GetClientResponse(LPCSTR ServerChallenge)
{
…
if (m_szPassword != NULL)
{
…
if (m_szPassword != '\0')
{
…
}
第二个if condition check意图检查m_szPassword是否为空字符串,但却错误的将指针与'\0'进行比较,正确的代码应该是这样的:
if (*m_szPassword != '\0')
[#2] Chromium项目 – NULL指针的处理
bool ChromeFrameNPAPI::Invoke(…)
{
ChromeFrameNPAPI* plugin_instance =
ChromeFrameInstanceFromNPObject(header);
if (!plugin_instance &&
(plugin_instance->automation_client_.get()))
return false;
…
}
一旦plugin_instance为NULL,!plugin_instance为True,代码对&&后面的子条件求值,引用plugin_instance将导致程序崩溃。正确的做法应该是:
if (plugin_instance &&
(plugin_instance->automation_client_.get()))
return false;
[#5] Apache httpd Server项目 – 不完整的缓冲区clear
#define MEMSET_BZERO(p,l) memset((p), 0, (l))
void apr__SHA256_Final(…, SHA256_CTX* context) {
…
MEMSET_BZERO(context, sizeof(context));
…
}
这个错误前面提到过,sizeof(context)只是指针的大小,将之改为sizeof(*context)就OK了。
[#7] PNG Library项目 – 意外的指针clear
png_size_t
png_check_keyword(png_structp png_ptr, png_charp key,
png_charpp new_key)
{
…
if (key_len > 79)
{
png_warning(png_ptr, "keyword length must be 1 – 79 characters");
new_key[79] = '\0';
key_len = 79;
}
…
}
new_key的类型为png_charpp,顾名思义,这是一个char**类型,但代码中new_key[79] = ‘\0′这句显然是要给某个char赋值,但new_key[n]得到的应该是一个地址,给一个地址赋值为’\0′显然是有误的。正确的写法应该是(*new_key)[79] = '\0'。
[#10] Miranda IM项目 – 保护没生效
void Append( PCXSTR pszSrc, int nLength )
{
…
UINT nOldLength = GetLength();
if (nOldLength < 0)
{
// protects from underflow
nOldLength = 0;
}
…
}
nOldLength椒UINT类型,其值永远不会小于0,因此if (nOldLength < 0)这行成了摆设。
[#12] Ultimate TCP/IP项目 – 不正确的循环结束条件
void CUT_StrMethods::RemoveSpaces(LPSTR szString) {
…
size_t loop, len = strlen(szString);
// Remove the trailing spaces
for(loop = (len-1); loop >= 0; loop–) {
if(szString[loop] != ' ')
break;
}
…
}
循环中的结束条件loop >= 0将永远为True,因为loop变量的类型是size_t是unsigned类型,永远不会小于0。
十、拷贝粘贴
和笔误不同,程序员们决不因该低估拷贝粘贴问题,这类问题发生了太多。程序员们花费了大量时间在这些问题的debug上。
[#1] Fennec Media Project项目 – 处理数组元素时出错
void* tag_write_setframe(char *tmem,
const char *tid, const string dstr)
{
…
if(lset)
{
fhead[11] = '\0';
fhead[12] = '\0';
fhead[13] = '\0';
fhead[13] = '\0';
}
…
}
咋看一下,fhead[13]做了两次赋值,似乎没啥问题。但仔细想一下,最后那行程序员的原意极可能是想写fhead[14] = '\0'。问题就在这里了。
[#2] MySQL项目 – 处理数组元素时出错
static int rr_cmp(uchar *a,uchar *b)
{
if (a[0] != b[0])
return (int) a[0] – (int) b[0];
if (a[1] != b[1])
return (int) a[1] – (int) b[1];
if (a[2] != b[2])
return (int) a[2] – (int) b[2];
if (a[3] != b[3])
return (int) a[3] – (int) b[3];
if (a[4] != b[4])
return (int) a[4] – (int) b[4];
if (a[5] != b[5])
return (int) a[1] – (int) b[5];
if (a[6] != b[6])
return (int) a[6] – (int) b[6];
return (int) a[7] – (int) b[7];
}
编写这类代码时,我猜绝大多数人会选择Copy-Paste,然后再逐行修改,问题就发生在修改过程中,上面的代码中当处理a[5] != b[5]时就忘记修改一个下标了:return (int) a[1] – (int) b[5];显然这里的正确代码应该是return (int) a[5] – (int) b[5]。
[#3] TortoiseSVN项目 文件名不正确
BOOL GetImageHlpVersion(DWORD &dwMS, DWORD &dwLS)
{
return(GetInMemoryFileVersion(("DBGHELP.DLL"),
dwMS,
dwLS)) ;
}
BOOL GetDbgHelpVersion(DWORD &dwMS, DWORD &dwLS)
{
return(GetInMemoryFileVersion(("DBGHELP.DLL"),
dwMS,
dwLS)) ;
}
GetImageHlpVersion和GetDbgHelpVersion都使用了"DBGHELP.DLL"文件,显然GetImageHlpVersion写错文件名了。应该用"IMAGEHLP.DLL"就对了。
[#4] Clang项目 – 等同的函数体
MapTy PerPtrTopDown;
MapTy PerPtrBottomUp;
void clearBottomUpPointers() {
PerPtrTopDown.clear();
}
void clearTopDownPointers() {
PerPtrTopDown.clear();
}
我们看到虽然两个函数名不同,但是函数体的内容是相同的,显然又是copy-paste惹的祸。做如下修改即可:
void clearBottomUpPointers() {
PerPtrBottomUp.clear();
}
十一、Null指针的校验迟了
这里的“迟了”的含义是先使用指针,然后再校验指针是否为NULL。
[#1] Quake-III-Arena项目 – 校验迟了
void Item_Paint(itemDef_t *item) {
vec4_t red;
menuDef_t *parent = (menuDef_t*)item->parent;
red[0] = red[3] = 1;
red[1] = red[2] = 0;
if (item == NULL) {
return;
}
…
}
在校验item是否为NULL前已经使用过item了,一旦item真的为NULL,那程序必然崩溃。
十二、其他杂项
[#1] Image Processing 项目 – 八进制数
inline
void elxLuminocity(const PixelRGBus& iPixel,
LuminanceCell< PixelRGBus >& oCell)
{
oCell._luminance = uint16(0.2220f*iPixel._red +
0.7067f*iPixel._blue + 0.0713f*iPixel._green);
oCell._pixel = iPixel;
}
inline
void elxLuminocity(const PixelRGBi& iPixel,
LuminanceCell< PixelRGBi >& oCell)
{
oCell._luminance = 2220*iPixel._red +
7067*iPixel._blue + 0713*iPixel._green;
oCell._pixel = iPixel;
}
第二个函数,程序员原意是使用713这个十进制整数,但0713 != 713,在C中,0713是八进制的表示法,Compiler会认为这是个八进制数。
[#2] IPP Sample工程 – 一个变量用于两个loop中
JERRCODE CJPEGDecoder::DecodeScanBaselineNI(void)
{
…
for(c = 0; c < m_scan_ncomps; c++)
{
block = m_block_buffer + (DCTSIZE2*m_nblock*(j+(i*m_numxMCU)));
// skip any relevant components
for(c = 0; c < m_ccomp[m_curr_comp_no].m_comp_no; c++)
{
block += (DCTSIZE2*m_ccomp
1
[/c]
.m_nblocks);
}
…
}
变量c用在了两个loop中,这会导致只有部分数据被处理,或外部循环中止。
[#3] Notepad++项目 – 怪异的条件表达式
int Notepad_plus::getHtmlXmlEncoding(….) const
{
…
if (langT != L_XML && langT != L_HTML && langT == L_
PHP)
return -1;
…
}
代码中的那行if条件等价于 if (langT == L_PHP),显然似乎不是作者原意,猜测正确的代码应该是这样的:
int Notepad_plus::getHtmlXmlEncoding(….) const
{
…
if (langT != L_XML && langT != L_HTML && langT != L_PHP)
return -1;
…
}