来自Coverity使用getenv的污点字符串消息

Question

在我的代码上运行Coverity会导致受污染的字符串错误消息.我正在使用堆栈中声明的"path"变量,所以我不确定为什么我会看到错误.我只能认为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));

Answer 1

不,这可能无法解决错误.

Coverity告诉你环境变量"A"中的数据几乎可以是任何东西; 此数据不受您的程序控制.

因此,在使用数据之前,需要对数据进行一些健全性检查.

如果有人将环境变量A设置为包含1025个字符的字符串,则建议的修复程序当前会有缓冲区溢出.

此外,任何版本的代码都不会NUL终止"路径"字符串.这是因为,你正在使用strncpy,如果应用了字节限制,它将不会NUL终止(在这种情况下,它会说"将复制的字符串限制为我从字符串中获得的长度").

您应该做的是首先检查字符串的大小.如果它太大,则返回某种错误代码; 变量A中的路径太大,因此您的代码将无法按预期运行.如果它不是太大,请将其复制到路径缓冲区中.如果你想使用strncpy,请确保在最后为NUL留出空间,然后明确地添加它,因为strncpy不能保证在那里放置NUL.

Answer 2

您的代码不正确:两种备选方案都有潜在的缓冲区溢出.

我不确定Coverity是否正确诊断了问题,您没有发布确切的错误消息.Coverity可能表示您使用了环境中的字符串,该字符串可能具有任何长度,当您的代码复制到1024字节缓冲区时可能导致缓冲区溢出,实际上它指向您这是一件好事.原因如下:

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

你的代码:

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 as ((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中的污染字符串来看,似乎Coverity对于受污染的字符串有点极端.虽然你得到的警告表明一个真正的问题,但摆脱警告有时可能需要奇怪的解决方法.