2016-09-06 6 views
0

Я использую инструмент SA покрытия для ошибок. Я получаю несколько ошибок из-за использования fgets(). Это сниппет (ошибки SA, представленные в виде комментариев) -Возможная уязвимость безопасности при использовании fgets() и рекомендуемого решения?

FILE *fp; 
char my_pubkey[1024]; 

fp = fopen("publickey.pub", "r"); 

//tainted_string_argument: fgets taints variable my_pubkey. 
if (!fgets(my_pubkey, sizeof(my_pubkey), fp)) { 
    printf("failure to read pub key file"); 
    goto error; 
} 

//tainted_string: Passing tainted string my_pubkey to a parameter that cannot accept a tainted format string. 
if (fprintf(fp, my_pubkey) != strlen(my_pubkey)) { 
    printf ("failure to write pub key in key file"); 
    goto error; 
} 

В моем исследовании, некоторые отчеты предлагают использовать GetLine() вместо этого, но что на самом деле нужно? Если это допустимая проблема, какова может быть уязвимость? И какое лучшее решение?

EDIT: Если это ложный позитив, почему так? Что может быть примером того, когда это будет актуальной проблемой?

+1

'' strlen' и printf' в его нынешнем виде может вызвать проблемы при работе с «испорченной» строки, если они являются (если нулевое завершение не там, где оно ожидается). Но это не ваше дело. 'fgets' заботится об этом. –

+1

Собственно, нет, извините. Это * не * ok. Что произойдет, если жало содержит некоторые спецификаторы формата? Так что нет, инструмент прав. Это настоящий позитив. –

+0

'fgets' добавляет символ новой строки, если встречается перед чтением' sizeof (my_pubkey) -1' chars. Может ли это быть проблемой для вашего приложения? –

ответ

3

Coverity, кажется, жалуются, что my_pubkey получает значение от внешнего источника. Поэтому он «испорчен», потому что программа не может по своей сути быть уверенной в том, что полученные таким образом данные являются правильными или действительными. Это настоящая забота о том, что вам просто нужно управлять. Я бы не ожидал использовать getline() вместо fgets(), чтобы изменить это - это будет один из способов решения другой проблемы, связанной с другой функцией (gets()).

Coverity является также жалуется, что вы передаете свою испорченную строку printf()как формат строка. Это также относится к проблеме безопасности, и, возможно, даже к простой правильной функциональности. Очень плохая идея использовать строку, поставляемую извне, как строку формата [f]printf(), потому что такая строка может содержать коды полей . Вы должны вместо этого либо предоставить явный формат:

fprintf(fp, "%s", my_pubkey) 

или использовать fputs():

fputs(my_pubkey, fp) 
2

Предполагая, что текст имеет текст до 1024 печатных символов, тогда код должен использовать char my_pubkey[1024+ 2];, чтобы освободить место для \n и \0.

Если ключ двоичный, то открытие в текстовом режиме является неправильным, и с использованием fgets() происходит сбой, так как он считывает строку . Лучше использовать fgetc(). Также не используйте strlen() как код, не имеющий отношения к строки.

Кроме того, внутренний Длина буфера должна быть минимизирована, чтобы уменьшить количество копий ключа, плавающего вокруг в устаревших буферах. См. setbuf() и setvbuf().


fprintf() интерпретирует строку, как и кодируется, как строка формата. @Eugene Sh.. Присутствие % вводит потенциал UB. Лучше использовать %s.

if (fprintf(fp, "%s", my_pubkey) ... 
Смежные вопросы