Why is this code vulnerable to buffer overflow attacks?

int func(char* str)
{
char buffer[100];
unsigned short len = strlen(str);


if(len >= 100)
{
return (-1);
}


strncpy(buffer,str,strlen(str));
return 0;
}

This code is vulnerable to a buffer overflow attack, and I'm trying to figure out why. I'm thinking it has to do with len being declared a short instead of an int, but I'm not really sure.

Any ideas?

29601 次浏览

在大多数编译器上,unsigned short的最大值是65535。

任何高于这个值的值都会被包装,所以65536变成0,65600变成65。

这意味着长度正确的长字符串(例如65600)将通过检查,并溢出缓冲区。


使用 size_t存储 strlen()的结果,而不是 unsigned short,并将 len与直接编码 buffer大小的表达式进行比较。例如:

char buffer[100];
size_t len = strlen(str);
if (len >= sizeof(buffer) / sizeof(buffer[0]))  return -1;
memcpy(buffer, str, len + 1);

即使使用 strncpy,截止值的长度仍然取决于传递的字符串指针。您不知道该字符串有多长(即空终止符相对于指针的位置)。因此,单独调用 strlen将使您暴露于脆弱性之中。如果你想更安全,使用 strnlen(str, 100)

更正后的完整代码如下:

int func(char *str) {
char buffer[100];
unsigned short len = strnlen(str, 100); // sizeof buffer


if (len >= 100) {
return -1;
}


strcpy(buffer, str); // this is safe since null terminator is less than 100th index
return 0;
}

问题在于:

strncpy(buffer,str,strlen(str));
^^^^^^^^^^^

如果字符串大于目标缓冲区的长度,strncpy 仍将复制它。您将字符串的字符数作为要复制的数字,而不是缓冲区的大小。做到这一点的正确方法如下:

strncpy(buffer,str, sizeof(buff) - 1);
buffer[sizeof(buff) - 1] = '\0';

这样做的目的是将复制的数据量限制为缓冲区的实际大小减去空终止字符的1。然后,我们将缓冲区中的最后一个字节设置为 null 字符,作为额外的保护措施。原因是,如果 strlen (str) < len-1,strncpy 将复制多达 n 个字节,包括终止 null。如果没有,则不会复制 null,并且会出现崩溃场景,因为现在缓冲区中有一个未终止的字符串。

希望这个能帮上忙。

编辑: 经过进一步审查和其他人的输入,该功能的可能编码如下:

int func (char *str)
{
char buffer[100];
unsigned short size = sizeof(buffer);
unsigned short len = strlen(str);


if (len > size - 1) return(-1);
memcpy(buffer, str, len + 1);
buffer[size - 1] = '\0';
return(0);
}

由于我们已经知道字符串的长度,因此可以使用 memcpy 将字符串从 str 引用的位置复制到缓冲区中。注意,对于 strlen (3)(在 FreeBSD 9.3系统上)的手册页,如下所述:

 The strlen() function returns the number of characters that precede the
terminating NUL character.  The strnlen() function returns either the
same result as strlen() or maxlen, whichever is smaller.

我将其解释为字符串的长度不包括 null。这就是为什么我复制 len + 1字节来包含 null,并且测试检查以确保长度 < size of buffer-2。减1是因为缓冲区从位置0开始,减1是为了确保有空间存放空值。

编辑: 事实证明,某物的大小以1开始,而 access 以0开始,所以之前的 -2是不正确的,因为它会返回一个大于98字节的错误,但它应该大于99字节。

编辑: 虽然关于无符号短符号的答案通常是正确的,因为可以表示的最大长度是65,535个字符,但这并不重要,因为如果字符串比这个长,值就会绕过去。这就像是取出75,231位(即0x000125DF) ,屏蔽掉前16位的9695位(即0x000025DF)。我看到的唯一问题是前100个字符大于65,535,因为长度检查允许复制 但是在所有情况下,它只复制字符串的前100个字符,并且 null 将终止字符串。因此,即使存在包装问题,缓冲区仍然不会溢出。

根据字符串的内容和使用它的目的,这本身可能会或不会构成安全风险。如果只是直接的文本,是人类可读的,那么一般没有问题。你只会得到一个截断的字符串。但是,如果是类似于 URL 甚至是 SQL 命令序列的东西,则可能会出现问题。

包装的答案是正确的。但是有一个问题我认为没有被提及 如果(len > = 100)

如果 Len 是100,我们会复制100个元素,我们不会有尾随的0。这显然意味着任何依赖于正确结束字符串的其他函数都将超出原始数组的范围。

从 C 语言出现的字符串问题是无法解决的。你最好在打电话之前有一些限制,但即使这样也没有用。没有边界检查,所以缓冲区溢出总是可能而且不幸地将会发生... ..。

除了多次调用 strlen所涉及的安全问题之外,通常不应该对长度精确已知的字符串使用字符串方法[对于大多数字符串函数来说,只有在非常狭窄的情况下才应该使用字符串方法——对于可以保证最大长度但不知道精确长度的字符串]。一旦输入字符串的长度已知,输出缓冲区的长度也已知,就应该计算出应该复制多大的区域,然后使用 memcpy()实际执行有问题的复制。尽管在复制只有1-3字节左右的字符串时,strcpy的性能有可能优于 memcpy(),但在许多平台上,当处理较大的字符串时,memcpy()的速度可能是 memcpy()的两倍以上。

尽管在某些情况下,安全性会以性能为代价,但是在这种情况下,安全性方法是 还有更快。在某些情况下,如果提供输入的代码可以确保输入的行为良好,并且如果防止行为不良的输入会阻碍性能,那么编写对行为异常的输入不安全的代码可能是合理的。确保只检查一次字符串长度可以提高 都有的性能和安全性,尽管在手动跟踪字符串长度时还可以做一件额外的事情来帮助保护安全性: 对于每个预期有尾随 null 的字符串,显式地写尾随 null,而不是预期源字符串有尾随 null。因此,如果一个人正在写一个 strdup等价物:

char *strdupe(char const *src)
{
size_t len = strlen(src);
char *dest = malloc(len+1);
// Calculation can't wrap if string is in valid-size memory block
if (!dest) return (OUT_OF_MEMORY(),(char*)0);
// OUT_OF_MEMORY is expected to halt; the return guards if it doesn't
memcpy(dest, src, len);
dest[len]=0;
return dest;
}

注意,如果 memcpy 已经处理了 len+1字节,那么通常可以省略最后一条语句,但是如果另一个线程要修改源字符串,那么结果可能是一个以 NUL 结尾的目标字符串。