Michael Howard
Secure Windows Initiative
摘要:Michael Howard 研究了一种常常被忽略的代码构造,这种构造可能会导致严重的缓冲区溢出问题,然后介绍了一种在没有溢出副作用的情况下执行算术运算的替代方法。
谈谈构造
很奇怪,有如此之多的安全指导文档提示人们注意危险的函数。在 C 和 C++ 中,很少有危险的函数,不过,有一件事是肯定的,有许多危险的开发人员正在使用 C 和 C++。
因此,您可能会问,“Michael,您究竟要讨论什么?”
我得承认,我听腻了一些文档说的所谓某些函数是危险的,您应该使用更安全的类型来代替它们。例如,“不要使用 strcpy,它是危险的。您应该改用 strncpy,因为它是安全的。”没有什么比这更远离实际情况的了。有可能使用 strcpy 的代码是安全的,而调用 strncpy 的却是不安全的代码。
像 strcpy 这样的函数是有潜在 危险的,因为源数据比目标缓冲区大,并且它来自不受信任的源。如果源数据来自一个受信任的源,并且在复制之前经过了有效性测试,则调用 strcpy 就是安全的:
void func(char *p) { const int MAX = 10; char buf[MAX + 1]; memset(buf,0,sizeof(buf)); if (p && strlen(p) <= MAX) strcpy(buf,p); }
信不信由您,我正好要在某处用到这个例子。有一种常常被忽略的构造可能会导致缓冲区溢出,它不是函数调用。它是这样的:
while () *d++ = *s++;
此处没有函数调用,这是 DCOM 中导致出现 Blaster worm 蠕虫病毒的编码构造。在 Buffer Overrun In RPC Interface Could Allow Code Execution 中,您可以读到更多关于此病毒的修复程序的内容。
该代码如下所示:
HRESULT GetMachineName(WCHAR *pwszPath) { WCHAR wszMachineName[N + 1]) LPWSTR pwszServerName = wszMachineName; while (*pwszPath != L'//' ) *pwszServerName++ = *pwszPath++; ... }
这里的问题在于,while 循环是以源字符串中的一些字符为界的。它没有为目标缓冲区的大小所限制。换句话说,如果源数据不受信任,就会出现缓冲区溢出。
我编写了一段简单的 Perl 脚本来搜索 C 和 C++ 代码中这些类型的构造。请注意,这段脚本标记的每个实例并不是一个缺陷,您需要确定是否源数据是受信任的。
use strict; use File::Find; my $RECURSE = 1; ################################################### foreach(@ARGV) { next if /^-./; if ($RECURSE) { finddepth(/&processFile,$_); } else { find(/&processFile,$_); } } ################################################### sub processFile { my $FILE; my $filename = $_; if (!$RECURSE && ($File::Find::topdir ne $File::Find::dir)) { $File::Find::prune = 1; return; } # Only accept C/C++ and header extensions return if (!(//.[ch](?:pp|xx)?$/i)); warn "$!/n" unless open FILE, "<" . $filename; # reset line number $. = 0; while () { chomp; s/^/s+//; s//s+$//; if (//*/w+/+/+/s{0,}=/s{0,}/*/w+/+/+) { print $filename . " " . $_ . "/n"; } }
注这段脚本只查找 *p++ 构造,而不查找 *++p 构造。
假定您发现了一个缺陷,使代码更安全的一种方法是限制被复制的数据不大于目标缓冲区:
HRESULT GetMachineName(WCHAR *pwszPath) { WCHAR wszMachineName[N + 1]) LPWSTR pwszServerName = wszMachineName; size_t cbMachineName = N; while (*pwszPath != L'//' && --cbMachineName) *pwszServerName++ = *pwszPath++; ... }
最后,对不为目标缓冲区的大小所限制的任何内存复制函数或构造都应该进行严格检查。
关于整数溢出的更多介绍
在前面的文章 Reviewing Code for Integer Manipulation Vulnerabilities 中,我讨论了与所谓整数溢出 的简单数学运算相关的安全性缺陷。
最近,作为正在进行的可信赖计算工程系列 (Trustworthy Computing Engineering Series) 的一部分,我给 Microsoft 的工程师做了一次关于整数溢出的讲座。在讲座中,我概述了如何发现整数溢出以及如何修复整数溢出。让我感到吃惊的是,我接收到的许多电子邮件都说我的补救方法很好,但是充满危险。请允许我做一些解释。
在该专栏中,我提到过的代码如下所示:
if (A + B > MAX) return -1;
应该改成这样:
if (A + B >= A && A + B < MAX) { // cool! }
三年前就有人指出,一些人会看到这段代码,但是不知道它有什么用,从而可能删除 A+B >= A 部分,因为它看起来纯属多余,而现在,整数溢出又重新出现在您面前。不会吧!
作为回应,我写了下面的头文件,它的意图再明白不过了。是的,它看起来像乱码,但这段乱码却是 x86 汇编语言。我之所以使用汇编语言,是因为它可以使我直接访问 jc 操作数,即按进位转移 (jump-on-carry)。换句话说,它检测数学运算是否会导致溢出。
#ifndef _INC_INTOVERFLOW_ #define _INC_INTOVERFLOW_ #ifdef _X86_ inline bool UAdd(size_t a, size_t b, size_t *r) { __asm { mov eax,dword ptr [a] add eax,dword ptr [b] mov ecx,dword ptr [r] mov dword ptr [ecx],eax jc short j1 mov al,1 jmp short j2 j1: #ifdef _DEBUG int 3 #endif xor al,al j2: }; } inline bool UMul(size_t a, size_t b, size_t *r) { __asm { mov eax,dword ptr [a] mul dword ptr [b] mov ecx,dword ptr [r] mov dword ptr [ecx],eax jc short j1 mov al,1 jmp short j2 j1: #ifdef _DEBUG int 3 #endif xor al,al j2: }; } inline bool UMulAdd(size_t mul1, size_t mul2, size_t add, size_t *r) { size_t t = 0; if (UMul(mul1,mul2,&t)) return UAdd(t,add,r); return false; } #else # error "This code compiles only on 32-bit x86 #endif // _X86_ #endif // _INC_INTOVERFLOW_
请查看这个文件,它包括解释这些函数的简单文档。
发现安全漏洞
虽然花了一点时间,但是许多人都找到了漏洞。当通过比较字符串来做出安全决策时,这种比较就应该是区域性不变的比较或字节方式的比较。在这个例子中,我编写的代码可能允许访问土耳其语的敏感数据,因为在土耳其语中,字母“I”有四个实例,两个小写字母,两个大写字母。您可以在 http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpref/html/frlrfsystemstringclasscomparetopic5.asp 上阅读这方面的内容。
现在,让我们转到本月的错误。这段代码有什么问题?
void func(char *p) { char buf[10+1]; memset(buf,0,sizeof(buf)); // limit string to 10 chars sprintf(buf,"%10s",p); printf("Hello, %s/n",buf); }
一个小智力游戏
这个智力游戏实际上与安全性毫无关系,但是当我想到人们为整数溢出检测代码所困扰时,我就把它从我记忆的深处拉了出来。这段代码有什么用呢?
int a = 0x42; int b = 0x69;
a ^= b; b ^= a; a ^= b;
这个游戏的规则非常简单,您无法编译或解释这段代码。试着仅仅通过观察来确定它有什么用。
Michael Howard 是 Microsoft Secure Windows Initiative 组的高级安全程序经理,是 Writing Secure Code 的合著者之一,该书的第二版现已发行。他还是 Designing Secure Web-based Applications for Windows 2000 的主要作者。他致力于确保人们所设计、构建、测试和记录的系统符合安全要求。他最喜欢的一句话是“一人之工具,他人之凶器。”
Michael Howard
Secure Windows Initiative
April 28, 2003
Summary: Michael Howard dishes on integer manipulation vulnerabilities and outlines a security plan that you can use to safeguard your own applications. (8 printed pages)
A couple of years ago, very few people had heard of integer overflow attacks, and now it seems there's a new one every few days. Below is a short list of some integer overflow security bugs found in the last few months:
- Sun RPC xdr_array
- OpenSSH authentication
- Apache Chunked Encoding
- Microsoft JScript
- FreeBSD socket and system calls
- Snort TCP Packet Reassembly
In this month's column, I'll outline how these bugs occur, how you can hunt for them in your code, and how you can fix them.
Oh, before I get into the core of this article, I'm happy to announce that Writing Secure Code received the RSA Conference Award for Industry Innovation at the RSA Security Conference held in San Francisco, April 2003.
Okay, back to integer attacks.
I'm not going to explain what an integer is, I assume you know what they are, and you know that there are basically two flavors—signed and unsigned—where signed integers have the high-bit set to 1 when the value is negative, this is a consequence of 2's complement arithmetic. And you also know there are different size integers, the most common being 64-bit, 32-bit, 16-bit, and 8-bit in length. That's all I'm going to say about integers, and for the purposes of this paper, that's all you need know.
There are three main integer manipulations that can lead to security vulnerabilities:
- Overflow and underflow
- Signed versus unsigned errors
- Truncation
On their own, these issues may not cause security errors. However, if your code exhibits one or more of these issues and manipulates memory, the potential for a buffer overrun error or application failure increases. Let's look at each in detail.
Overflow and Underflow
Quick, what's wrong with this code?
bool func(size_t cbSize) { if (cbSize < 1024) { // we never deal with a string trailing null char *buf = new char[cbSize-1]; memset(buf,0,cbSize-1); // do stuff delete [] buf; return true; } else { return false; } }
The code is correct, right? It validates that cbSize
is no larger than 1 KB, and new
or malloc
should always allocate 1 KB correctly, right? Let's ignore the fact that the return value of new
or malloc
should be checked for the moment. Also, cbSize
cannot be a negative number, because it's a size_t
. But what if cbSize
is zero? Look at the code that allocates the buffer—it subtracts one from the buffer size request. Subtracting one from zero causes a size_t
variable, which is an unsigned integer, to wrap under to 0xFFFFFFFF (assuming a 32-bit value), or 4 GB. Your application just died—or worse!
Here's a similar issue:
bool func(char *s1, size_t len1, char *s2, size_t len2) { if (1 + len1 + len2 > 64) return false; // accommodate for the trailing null in the addition char *buf = (char*)malloc(len1+len2+1); if (buf) { StringCchCopy(buf,len1+len2,s1); StringCchCat(buf,len1+len2,s2); } // do other stuff with buf if (buf) free(buf); return true; }
Once again, the code looks well written; it checks the data sizes, it verifies that malloc
succeeded, and it uses the safe string handling functions, StringCchCopy
and
(you can read more about these string handling functions at http://msdn.microsoft.com/library/en-us/dnsecure/html/strsafe.asp). However, this code suffers from an integer overflow. What if StringCchCat
len1
is 64, and len2
is 0xFFFFFFFF? The code that determines the buffer size is legal adds 1, 64 and 0xFFFFFFFF together, which yields 64, because the addition operation wraps around. Next, the code allocates only 64 bytes, and the code then builds a new string of 64 bytes in length, and then concatenates 0xFFFFFFFFF bytes to the string. Once again, the application dies, and in some cases, some code when attacked with carefully crafted sizes can lead to exploitable buffer overrun attacks.
There's another lesson here —safe string handling functions are not safe if you get the buffer size calculation incorrect.
The JScript Overflow Attack
The same kind of overflow bug can occur when multiplying, which is what happened in the Microsoft JScript bug. The bug only manifests itself when using the JScript sparse array support:
var arr = new Array(); arr[1] = 1; arr[2] = 2; arr[0x40000001] = 3;
In this example, the array has three elements and a length of 0x40000001 (1073741825 decimal). But since this example uses sparse arrays, it only consumes the memory of a three-element array.
The C++ code that implements the JScript customized sort routine allocates a temporary buffer on the heap, copies the three elements into the temporary buffer, sorts the temporary buffer using the custom function, and then moves the contents of the temporary buffer back into the array. Here's the code that allocates the temporary buffer:
TemporaryBuffer = (Element *)malloc(ElementCount * sizeof(Element));
An Element
is a 20-byte data structure used to hold an array entry. It looks like the program will attempt to allocate around 20 GB for the temporary buffer. You might think that since most people don't have 20 GB of memory in their machines the allocation attempt will fail. Then the JScript regular out-of-memory handling routines will handle the problem. Unfortunately, that is not what happens.
When using 32-bit integer arithmetic, we get an integer overflow attack because the result (0x0000000500000014) is too large to hold in a 32-bit value:
0x40000001 * 0x00000014 = 0x0000000500000014
C++ throws away all the bits that don't fit, so we get 0x00000014. This is why the allocation does not fail—the allocation does not attempt to allocate twenty billion bytes, but rather it attempts to allocate only twenty bytes. The sort routine then assumes that the buffer is large enough to hold the three elements in the sparse array, so it copies the 60 bytes that make up the three elements into a 20 byte buffer, overrunning the buffer by 40 bytes. Oops!
Signed vs. Unsigned Errors
Take a quick look at the following code. It's similar to the first example. See if you can spot the error, and if you do, try to determine what the result is.
bool func(char *s1, int len1, char *s2, int len2) { char buf[128]; if (1 + len1 + len2 > 128) return false; if (buf) { strncpy(buf,s1,len1); strncat(buf,s2,len2); } return true; }
The problem here is the string sizes are stored as signed integers, so len1
can be larger than 128 as long as len2
is negative, hence the sum is less than 128 bytes. However, a call to strncpy
will overflow the buf
buffer.
Truncation Errors
Now let's look at the last attack type, by way of, you guessed it, a code example.
bool func(byte *name, DWORD cbBuf) { unsigned short cbCalculatedBufSize = cbBuf; byte *buf = (byte*)malloc(cbCalculatedBufSize); if (buf) { memcpy(buf, name, cbBuf); // do stuff with buf if (buf) free(buf); return true; } return false; }
The attack, or at least the result, is a little like the JScript bug outlined earlier. What if cbBuf
is 0x00010020? cbCalculatedBufSize
is only 0x20 because only the lower 16-bits from 0x00010020 are copied. Hence only 0x20 bytes are allocated, and 0x00010020 bytes are copied into the newly allocated target buffer. Note that compiling this code with Microsoft Visual C++® /W4 option yields:
warning C4244: 'initializing' : conversion from 'DWORD' to 'unsigned short', possible loss of data
Be aware that operations like the following do not flag a warning:
int len = 16; memcpy(buf, szData, len);
The last argument to memcpy
is a size_t
, yet the argument, len
, is signed. The warning is not issued because memcpy
always assumes the third argument is unsigned and putting a cast to unsigned would not change the function outcome.
Note you will get a warning if you attempt to assign a DWORD
with a size_t
, not because there is a potential loss of data on 32-bit platforms, but because there will be data loss on 64-bit platforms:
warning C4267: '=' : conversion from 'size_t' to 'DWORD', possible loss of data
You get this warning because all default C++ projects compile with the –Wp64 option, which tells the compiler to watch for 64-bit portability issues.
Integer Manipulation Issues in Managed Code
Integer manipulation errors can occur in managed languages, such as C# and Visual Basic® .NET, but the potential for damage is greatly reduced as your code does not have direct access to memory. However, calling into native code (assuming your code is granted the permission to call unmanaged code.) can still cause security issues like those noted above. Integers in the Common Language Specification (CLS) are signed, and one mistake is to validate signed integer arguments in managed code, when the variables are treated as unsigned integers in the unmanaged code.
This is a specific case of the more general advice of always check what you're passing to unmanaged code. Many integer manipulation bugs in managed code can cause reliability errors in Visual Basic .NET, because all such operations will raise the System.OverflowException
if an overflow or underflow occurs.
By default, C# does not throw these exceptions. Use the checked
keyword if you wish to check for these issues:
UInt32 i = 0xFFFFFFF0; UInt32 j = 0x00000100; UInt32 k; checked {k = i + j;}
Remedies
Who would have thought that simply manipulating integers could lead to security problems? A simple remedy to vulnerable code like this:
if (A + B > MAX) return -1;
Is to use this code with unsigned integers:
if (A + B >= A && A + B < MAX) { // cool! }
The first operation A + B >= A
, checks for the wrap-around, and the second makes sure the sum is less than the target size.
As for the multiplication issue in JScript, you can check that the number of elements is no larger than a predetermined value that is smaller than the largest amount of memory you are willing to allocate. For example, the following code could potentially allocate up to 64 MB:
const size_t MAX = 1024 * 1024 * 64; const size_t ELEM_SIZE = sizeof(ELEMENT); const size_t MAX_ELEMS = MAX / ELEM_SIZE; if (cElems >= MAX_ELEMS) return false;
Finally, use unsigned integers (such as DWORD and size_t) for array indexes, buffer sizes, and the like.
Key Code Reviewing Points
Keep the following in mind when compiling or reviewing code for integer-related issues:
- Compile C and C++ code with the highest possible warning level, /W4.
- Use size_t or DWORD for buffer sizes and element counts. There is no reason to use a signed value for such constructs.
- Keep in mind that a size_t is a different type depending on the platform you use. A size_t is the size of a memory address, so it is a 32-bit value on a 32-bit platform, but a 64-bit value on a 64-bit platform.
- If your code performs any kind of integer manipulation (addition, multiplication, and so on) where the result is used to index into an array or calculate a buffer size, make sure the operands fall into a small and well-understood range.
- Be wary of signed arguments to memory allocation functions (new, malloc, GlobalAlloc, and so on) because they are treated as unsigned integers.
- Watch out for operations that yield C4018, C4389, and C4244 warnings.
- Watch out for casts that cast away the C4018, C4389, and C4244 warnings.
- Investigate all use of #pragma warning(disable, Cnnnn) that disable the C4018, C4389, and C4244 warnings. In fact, comment them out, re-compile, and check all new integer-related warnings.
- Code migrated from other platforms or compilers may make different assumptions about data sizes. Watch out!
- If calling unmanaged code from managed code, make sure you confirm that it is signed correctly. Many arguments to Win32 APIs are unsigned ints or DWORDs, and many managed code variables are signed.
- Finally, if you are using managed code, make sure you catch OverflowExceptions, if appropriate.
Spot the Security Flaw
Many people worked out my last flaw. It was an integer overflow attack. So what's wrong with this C# code?
string Status = "No"; string sqlstring =""; try { SqlConnection sql= new SqlConnection( @"data source=localhost;" + "user id=sa;password=password;"); sql.Open(); sqlstring="SELECT HasShipped" + " FROM detail WHERE ID='" + Id + "'"; SqlCommand cmd = new SqlCommand(sqlstring,sql); if ((int)cmd.ExecuteScalar() != 0) Status = "Yes"; } catch (SqlException se) { Status = sqlstring + " failed/n/r"; foreach (SqlError e in se.Errors) { Status += e.Message + "/n/r"; } } catch (Exception e) { Status = e.ToString(); }
Michael Howard is a Senior Security Program Manager in the Secure Windows Initiative group at Microsoft and is the coauthor of Writing Secure Code, now in its second edition, and the main author of Designing Secure Web-based Applications for Windows 2000. His main focus in life is making sure people design, build, test, and document nothing short of a secure system. His favorite line is "One person's feature is another's exploit."