9

在我的代码上运行 Coverity 会导致受污染的字符串错误消息。我正在使用堆栈中声明的“路径”变量，所以我不确定为什么会看到错误。我只能认为getenv()直接在中使用strncpy()会导致错误。下面的修复程序会消除此错误吗？

char path[1024] = {NULL, };
if(getenv("A"))
    strncpy(path, getenv("A"), strlen(getenv("A")));

至

char path[1024] = {NULL, };
char * adriver = getenv("A");
if(adriver)
    strncpy(path, adriver, strlen(adriver));
4

4 回答 4

8

No, this will probably not fix the error.

Coverity is telling you that the data within environment variable "A" could be pretty much anything; this data is not under the control of your program.

Therefore, you need to have some sanity checks on the data before you use it.

Your proposed fix will currently have a buffer overflow, if somebody sets environment variable A to a string containing 1025 characters.

Additionally, neither version of code will ever NUL-terminate the "path" string. This is because, you are using strncpy, which will not NUL-terminate if the byte limit is applied (which it will in this case, because you say "limit the copied string to the length I just got from the string").

What you should be doing, is size-check the string first. If it is too large, return some sort of error code; the path in variable A is too big, so your code will not function as desired. If it is not too large, copy it into the path buffer. If you want to use strncpy, make sure to leave room for a NUL at the end, and then explicitly add it, since strncpy is not guaranteed to put a NUL there.

于 2014-12-17T17:38:18.827 回答
7

您的代码不正确：两种选择都有潜在的缓冲区溢出。

我不确定Coverity是否正确诊断问题，您没有发布确切的错误消息。Coverity 可能表明您使用了环境中的字符串，该字符串可能具有任意长度，当您的代码将其复制到 1024 字节缓冲区时，可能会导致缓冲区溢出，这确实是一件好事。原因如下：

strncpy不做你认为它做的事。永远不要使用这个函数，它的语义很容易出错，它不是适合你的工具。从tostrncpy(dest, src, n)复制不超过nchars并用字节填充数组的其余部分，直到字节被写入。必须指向至少包含字符的数组。如果更短，则行为效率低下，因为填充通常是不必要的，但如果长度至少为，则不会以 为空终止，在许多情况下会导致未定义的行为。srcdestdest'\0'ndestnsrcsrcndeststrncpy

你的代码：

char path[1024] = { NULL, };
if (getenv("A"))
    strncpy(path, getenv("A"), strlen(getenv("A")));

相当于

char path[1024] = { NULL, };
if (getenv("A"))
    memcpy(path, getenv("A"), strlen(getenv("A")));

如您所见，没有真正的保护被授予。

您调用getenv3 次，使用您的替代实现确实会更有效，但还有其他问题：

path你用初始化{ NULL, }。这是不一致的，在许多情况下是不正确的。 NULL通常 #defined 为((void*)0)，因此 . 的初始值设定项无效char。 path可以这样初始化：

char path[1024] = { 0 };

为避免溢出目标缓冲区，请使用以下代码：

char path[1024] = { 0 };
char *p = getenv("A");
if (p != NULL) {
    strncat(path, p, sizeof(path) - 1);
}

但这会截断环境值，这可能不合适，具体取决于您使用path.

另一种方法是直接使用环境值：

char *path = getenv("A");
if (path == NULL)
    path = "";

如果您setenv在程序执行期间更改环境值，您可能希望使用 复制环境值path = strdup(path);。这也可能修复来自 Coverity 的污染字符串警告，尽管副本的大小与原始的大小相同，并且可能没有足够的内存可用，应该对其进行测试。从C 中的 Tainted string来看，Coverity 似乎对 tainted strings 有点极端。虽然您收到的警告表明存在真正的问题，但消除警告有时可能需要奇怪的解决方法。

于 2016-01-30T11:10:15.263 回答
0

下面的修复程序会消除此错误吗？

不，但这几乎肯定会：

char *e = getenv("A");
char path[e ? strlen(e) + 1 : 1];
strcpy(path, e ? e : "");

当getenv("A")返回一个 1024 字节或更长的字符串时，您的代码会出现主要问题；按原样复制path会导致缓冲区溢出，因为不能保证strlen(getenv("A"))小于或等于 1024。您可以使用以下方法解决此问题：

strncpy(path, getenv("A"), sizeof path); // NOT RECOMMENDED

...但path不会以'\0'字符终止；path在这种情况下不会是字符串，因为根据定义，字符串必须以'\0'字符结尾......这意味着您将无法将其用作字符串。

这就是 Coverity 最有可能在这里抱怨的问题，我的代码通过使用一个足以存储整个字符串以及字符串'\0'末尾的 VLA（可变长度数组）来消除该问题。

于 2016-01-30T10:54:10.883 回答
-1 
char *path = getenv("A");

这不是你所需要的吗？您是否有理由需要复制 getenv 返回的内容？如果是这样，

char *path = NULL, *A = getenv("A");

if(A != NULL)
   {
   path = malloc(strlen(A)+1);
   strcpy(path, A);
   }
于 2016-01-30T02:11:52.273 回答