Tainted string message from Coverity using getenv

Question

Running Coverity on my code results in tainted string error message. I am using the "path" variable declared in the stack, so I am not sure why I am seeing errors. I can only think that using getenv() directly in the strncpy() is causing the error. Would the fix below eliminate this error?

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

to

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

Answer 1

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.