为什么这段代码容易受到缓冲区溢出攻击的影响?

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

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

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

这段代码容易受到缓冲区溢出攻击,我正在尝试弄清楚原因。我认为与将len声明为short而不是int有关,但我不确定。

有任何想法吗?


3
这段代码存在多个问题。请记得C字符串需要以空字符结尾。 - Dima Chubarov
4
只有在调用strncpy后继续使用该字符串时,不对字符串进行零结尾会成为一个问题。但在此情况下,没有这种情况发生。 - R Sahu
43
这段代码的问题直接源于strlen被计算后用于验证检查,然后荒谬地再次计算--这是DRY原则的失败。如果第二个strlen(str)被替换成len,那么无论len的类型是什么,都不会有缓冲区溢出的可能性。这些答案没有解决这一点,只是设法回避它。 - Jim Balter
3
如果传递给strlen的字符串没有以空字符结尾,它将显示未定义的行为。 - Kaiserludi
3
@JimBalter 不,我想我会把它们留在那里。也许其他人会有相同的愚蠢误解,并从中学习。如果它们让你不爽,请随时标记它们,有人可能会过来删除它们。 - Asad Saeeduddin
显示剩余7条评论
5个回答

196

大多数编译器中,unsigned short 的最大值为 65535。

超过这个值将被包装回到 0,比如 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);

2
@PatrickRoberts 理论上是可以的。但你必须记住,10%的代码负责90%的运行时间,所以在安全性之前不应该牺牲性能。并且要记住,随着时间的推移,代码会发生变化,这可能意味着之前的检查已经不存在了。 - orlp
3
为了防止缓冲区溢出,只需将 len 作为 strncpy 的第三个参数即可。在任何情况下再次使用 strlen 都是愚蠢的。 - Jim Balter
16
在C语言中,/ sizeof(buffer[0]) 表示除以数组 buffer 中元素的大小。需要注意的是,在C语言中,即使一个 char 包含了很多位,sizeof(char) 的大小始终为1,因此当没有使用不同的数据类型时,这个除号后面的部分是多余的。尽管如此,对于完整的回答表示赞赏(并感谢您对评论的及时响应)。 - Jim Balter
4
char[]char*并不是同样的东西。在许多情况下,char[]会自动转换为char*。例如,当作为函数参数类型时,char[]char*完全相同。但是,对于sizeof()操作符,不会发生这种转换。请注意,本人只翻译内容,不做任何解释或添加其他内容。 - Dietrich Epp
4
因为如果在某个时刻更改了buffer的大小,该表达式会自动更新。这对于安全至关重要,因为buffer的声明可能与实际代码中的检查相隔很远。因此,更改buffer的大小很容易,但很容易忘记在使用该大小的每个位置进行更新。 - orlp
显示剩余17条评论

28

问题就在这里:

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

如果字符串的长度大于目标缓冲区的长度,strncpy仍会将其复制到缓冲区中。你是以字符串的字符数作为要复制的数量,而不是缓冲区的大小。正确的做法如下:

strncpy(buffer,str, sizeof(buff) - 1);
buffer[sizeof(buff) - 1] = '\0';
这样做的作用是限制复制到缓冲区实际大小减去一个空终止字符的数据量。然后,我们将缓冲区中的最后一个字节设置为空字符,作为额外的保障措施。这样做的原因是,如果strlen(str) < len - 1,则strncpy将复制n个字节,包括终止空字符。否则,空字符不被复制,您将面临崩溃场景,因为现在您的缓冲区具有未终止的字符串。
希望这可以帮助您。
编辑:经过进一步检查和他人的建议,该函数的可能编码如下:
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引用的位置复制到缓冲区中。请注意,根据FreeBSD 9.3系统上strlen(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.
我理解为字符串的长度不包括空字符,所以我复制 len+1 个字节来包含空字符,测试将检查长度是否小于缓冲区大小减2。减一是因为缓冲区从位置0开始,再减一个确保有空间放置空字符。
编辑:结果发现,某些东西的大小从1开始,而访问则从0开始,所以之前的-2是错误的,因为它会返回任何大于98个字节的错误,但实际上应该是>99个字节。
编辑:尽管无符号短整型的答案通常是正确的,因为可以表示的最大长度为65,535个字符,但这并不重要,因为如果字符串超过该长度,值将会回绕。就像取75,231(即0x000125DF)并屏蔽掉前16位,得到9695(0x000025DF)。我唯一看到的问题是超过65,535个字符后的前100个字符,长度检查将允许复制,但在所有情况下只会复制字符串的前100个字符并对字符串进行空终止处理。因此,即使出现回绕问题,缓冲区仍然不会溢出。
这本身可能或可能不构成安全风险,这取决于字符串的内容以及您用它做什么。如果只是纯文本,那么通常没有问题。您只需获得一个被截断的字符串。但是,如果它是类似于URL或甚至是SQL命令序列之类的东西,您可能会遇到问题。

2
真的,但这超出了问题的范围。代码清楚地显示了函数被传递一个字符指针。在函数的范围之外,我们不关心。 - Daniel Rudy
“the buffer in which str is stored” - 这不是缓冲区溢出,这就是问题所在。而且每个答案都有这个“问题”,这是由于func的签名所决定的,也是每个接受以空字符结尾的字符串作为参数的C函数的通病。提出输入可能没有以空字符结尾的可能性完全是无知的表现。 - Jim Balter
“这超出了问题的范围” - 遗憾的是,有些人无法理解。 - Jim Balter
问题在这里--你是对的,但你仍然错过了关键问题,即测试(len >= 100)只针对一个值进行了,但复制的长度却给了另一个值...这违反了DRY原则。简单地调用strncpy(buffer, str, len)避免了缓冲区溢出的可能性,并且比strncpy(buffer,str,sizeof(buffer) - 1)做更少的工作...虽然在这里它只是memcpy(buffer, str, len)的较慢等价物。 - Jim Balter
“一般的编码实践说”——其实并不是这样,还有其他情况,这就是其中之一。“代码是安全的”——我没有说过不安全。“我坚持我的先前声明”——我没有质疑任何先前的声明。相反,你没有注意到我写的内容。 - Jim Balter
显示剩余4条评论

11
即使你在使用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;
}

@user3386109 那么 strlen 也会访问缓冲区的末尾吗? - Patrick Roberts
orlp的回答是正确的。我认为这个答案没有添加任何内容,可以被删除。这个答案忽略了OP的代码试图检查字符串的长度这一事实。一个完整的答案应该解释为什么这个检查不起作用。 - David Grayson
2
@user3386109,您所指出的问题使得orlp的答案与我的一样无效。如果orlp所建议的是正确的,我不明白为什么“strnlen”不能解决这个问题。 - Patrick Roberts
1
“我不认为strnlen在这里解决了任何问题” - 当然它有用;它可以防止buffer溢出。 “因为str可能指向2字节的缓冲区,其中没有一个是NUL。” - 这与func任何实现都无关。这里的问题是缓冲区溢出,而不是UB,因为输入没有以NUL结尾。 - Jim Balter
1
传递给strnlen的第二个参数必须是第一个参数指向的对象的大小,否则strnlen毫无意义--这完全是胡说八道。如果strnlen的第二个参数是输入字符串的长度,则strnlen等同于strlen。你怎么会得到那个数字,如果你有了它,为什么还需要调用str[n]len?这根本不是strnlen的用途。 - Jim Balter
1
虽然这个答案并不完美,因为它与提问者的代码不等价--strncpy会填充NUL,并且不会以NUL结尾,而strcpy则以NUL结尾,并且不会填充NUL。但是,它确实解决了问题,与上面荒谬无知的评论相反。 - Jim Balter

4
答案正确,但我认为存在一个问题没有被提及。如果长度大于等于100,则会复制100个元素,并且末尾不会有\0。这显然意味着任何其他依赖于正确结束字符串的函数都会超出原始数组。

从C语言来看,字符串问题是无法解决的。在调用之前最好设置一些限制,但即使如此也无济于事。没有边界检查,因此缓冲区溢出总是可能发生,而且不幸的是经常发生......


字符串问题是可以解决的:只需使用适当的函数。即不要使用strncpy()等函数,而是使用像strdup()等内存分配函数。它们在POSIX-2008标准中,因此它们相当可移植,尽管在某些专有系统上不可用。 - cmaster - reinstate monica
你的观察在我看来是错误的。if (len >= 100)是检查失败的条件,而不是通过的条件,这意味着没有一种情况下会恰好复制100个字节且没有NUL终止符,因为该长度已包含在失败条件中。 - Patrick Roberts
@cmaster。在这种情况下,你是错误的。它是不可解决的,因为人们总是可以超出边界进行编写。是的,这是未定义的行为,但没有办法完全防止它。 - Friedrich
@Jim Balter。没关系。我有可能会写超出这个本地缓冲区的边界,因此它总是可能会破坏其他数据结构。 - Friedrich
非正常情况。你似乎忘记了你写了什么,以及我在回应什么。 - Jim Balter
显示剩余3条评论

3
除了与多次调用strlen相关的安全问题外,在大多数字符串函数中,通常不应对长度已知的字符串使用字符串方法[只在极少数情况下可以使用它们,即对于可以保证最大长度但不确定精确长度的字符串]。一旦知道输入字符串的长度和输出缓冲区的长度,就应该找出应该复制多大的区域,然后使用memcpy()来执行所需的复制操作。虽然strcpy在仅复制1-3个字节的字符串时可能比memcpy()更快,但在处理较大字符串时,在许多平台上memcpy()可能会快两倍以上。
尽管有些情况下,安全可能会以性能为代价,但这是一个安全方法也是更快的方法。在某些情况下,编写代码可能不安全,而输入代码可以确保它们将表现良好,并且如果防范不端行为会影响性能,则可以合理地编写代码。确保字符串长度仅检查一次会提高性能和安全性,但即使手动跟踪字符串长度也可以采取一些额外措施来帮助防范安全风险:对于每个预计应该有结尾空字符的字符串,请明确写入结尾空字符,而不是期望源字符串具有该字符。因此,如果要编写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终止的目标字符串。

3
请问您能否说明调用 strlen 多次涉及的安全问题? - Bogdan Alexandru
1
@BogdanAlexandru:一旦调用了strlen并根据返回的值采取了某些操作(这可能是首次调用它的原因),那么重复调用要么(1)总是产生与第一个调用相同的答案,这样就浪费了工作,要么(2)有时会产生不同的答案(因为其他东西——也许是另一个线程——在此期间修改了字符串),在这种情况下,执行一些长度相关的代码(例如分配缓冲区)可能会假设与执行其他操作的代码(将其复制到缓冲区)不同的大小。 - supercat

网页内容由stack overflow 提供, 点击上面的
可以查看英文原文,
原文链接