Загрязненное строковое сообщение от Coverity с использованием getenv

Question

Запуск 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));

Answer 1

Нет, это, вероятно, не исправит ошибку.

Coverity говорит вам, что данные в переменной окружения "A" могут быть чем угодно; эти данные не находятся под контролем вашей программы.

Поэтому вам необходимо провести некоторые проверки работоспособности данных, прежде чем использовать их.

Предлагаемое исправление в настоящее время будет иметь переполнение буфера, если кто-то установит переменную среды A в строку, содержащую 1025 символов.

Кроме того, ни одна из версий кода никогда не завершит NUL-строку "path". Это потому, что вы используете 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 не будет нулевым 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")));

Как видите, никакой реальной защиты не предоставляется.

Ты звонишь getenv В 3 раза было бы более эффективно использовать альтернативную реализацию, но есть и другие проблемы:

Вы инициализируете path с { NULL, }, Это противоречиво и во многих случаях неверно. NULL обычно # определяется как ((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 немного экстремальна в испорченных струнах. Хотя полученное предупреждение указывает на реальную проблему, для его устранения иногда могут потребоваться странные обходные пути.

Answer 3

Устранит ли приведенное ниже исправление эту ошибку?

Нет, но это почти наверняка

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, скорее всего, жалуется, и мой код устраняет проблему, используя VLA (массив переменной длины), достаточно большой для хранения всей строки, плюс '\0' в конце строки.

Answer 4

char *path = getenv("A");

Разве это не все, что вам нужно? Есть ли причина, по которой вам нужно сделать копию того, что возвращает getenv? Если так,

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

if(A != NULL)
   {
   path = malloc(strlen(A)+1);
   strcpy(path, A);
   }