Проверка библиотеки Network Security Services
Network Security Services (NSS) — это набор библиотек, разработанных для поддержки кроссплатформенной разработки защищенных клиентских и серверных приложений. Библиотека используется для криптографии в браузерах Firefox и Chrome, и после недавно найденной уязвимости в проверке подписей сертификатов, я решил тоже взглянуть на этот проект.Подробнее об уязвимости.Исходный код был получен следующими командами:
Так как библиотека собирается из консоли Windows, то для проверки я воспользовался специальной утилитой PVS-Studio Standalone, которая описана в статье: PVS-Studio теперь поддерживает любую сборочную систему на Windows и любой компилятор. Легко и «из коробки».Результаты проверкиV547 Expression 'dtype!= 2 || dtype!= 3' is always true. Probably the '&&' operator should be used here. crlgen.c 1172 static SECStatus crlgen_setNextDataFn_field (…, unsigned short dtype) { … if (dtype!= CRLGEN_TYPE_DIGIT || //<== dtype != CRLGEN_TYPE_DIGIT_RANGE) { //<== crlgen_PrintError(crlGenData->parsedLineNum, «range value should have » «numeric or numeric range values.\n»); return SECFailure; } … } Из таблицы истинности логического сложения следует, что если хотя бы один операнд — единица (как в этом случае), то условие будет всегда истинным.V567 Undefined behavior. The 'j' variable is modified while being used twice between sequence points. pk11slot.c 1934
PK11SlotList* PK11_GetAllTokens (…) { … #if defined (XP_WIN32) waste[ j & 0xf] = j++; #endif … } Переменная 'j' дважды используется в одной точке следования. В результате невозможно предсказать результат работы такого выражения. Подробнее в описании диагностики V567.V575 The null pointer is passed into 'fclose' function. Inspect the first argument. certcgi.c 608
static int get_serial_number (Pair *data) { FILE *serialFile; … serialFile = fopen (filename, «r»); if (serialFile!= NULL) { … } else { fclose (serialFile); //<== .... } .... } В данном случае, если указатель на файл нулевой, то закрывать файл не надо. А вот если его закрывать, то это может привести к неприятностям. Что именно произойдет, зависит от установленного обработчика таких событий (см. _set_invalid_parameter_handler() и так далее).V576 Incorrect format. A different number of actual arguments is expected while calling 'fprintf' function. Expected: 3. Present: 7. pp.c 34
static void Usage (char *progName) { … fprintf (stderr,»%-14s (Use either the long type name or » «the shortcut.)\n»,», SEC_CT_CERTIFICATE_ID, SEC_CT_PKCS7, SEC_CT_CRL, SEC_CT_NAME); … } Количество спецификаторов формата не соответствует количеству передаваемых аргументов функции fprintf ().V595 The 'buf' pointer was utilized before it was verified against nullptr. Check lines: 1709, 1710. prtime.c 1709
PR_IMPLEMENT (PRUint32) PR_FormatTime (…) { … rv = strftime (buf, buflen, fmt, ap); if (! rv && buf && buflen > 0) { buf[0] = '\0'; } return rv; } Указатель 'buf' всё-таки проверяют на равенство нулю, значит, в предыдущей строчке возможно возникновение ошибки при передаче нулевого указателя функции strftime ().V597 The compiler could delete the 'memset' function call, which is used to flush 'hashed_secret' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. alghmac.c 87
#define PORT_Memset memset
SECStatus HMAC_Init (HMACContext * cx, const SECHashObject *hash_obj, const unsigned char *secret, unsigned int secret_len, PRBool isFIPS) { … PORT_Memset (hashed_secret, 0, sizeof hashed_secret); //<== if (cx->hash!= NULL) cx→hashobj→destroy (cx→hash, PR_TRUE); return SECFailure; } Самое опасное место для кода, отвечающего за обработку секретной информации. Так как массив 'hashed_secret' больше не используется после вызова функции 'memset', то компилятор может удалить вызов функции для оптимизации, и массив не будет обнулён, как планировалось.Это, пожалуй, самые серьезные ошибки из всех найденных.
Часто предупреждение V597 остаётся непонятым. Предлагаю дополнительные материалы, раскрывающие суть проблемы:
Список всех таких мест: V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory () function should be used to erase the private data. sha512.c 503 V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory () function should be used to erase the private data. sha512.c 605 V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory () function should be used to erase the private data. sha512.c 1307 V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory () function should be used to erase the private data. sha512.c 1423 V597 The compiler could delete the 'memset' function call, which is used to flush 'cx' object. The RtlSecureZeroMemory () function should be used to erase the private data. md5.c 209 V597 The compiler could delete the 'memset' function call, which is used to flush 'ctx' object. The RtlSecureZeroMemory () function should be used to erase the private data. sha_fast.c 416 V597 The compiler could delete the 'memset' function call, which is used to flush 'lastBlock' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. cts.c 141 V597 The compiler could delete the 'memset' function call, which is used to flush 'lastBlock' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. cts.c 299 V597 The compiler could delete the 'memset' function call, which is used to flush 'data' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. drbg.c 300 V597 The compiler could delete the 'memset' function call, which is used to flush 'inputhash' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. drbg.c 450 V597 The compiler could delete the 'memset' function call, which is used to flush 'localDigestData' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. dsa.c 417 V597 The compiler could delete the 'memset' function call, which is used to flush 'U' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. pqg.c 422 V597 The compiler could delete the 'memset' function call, which is used to flush 'sha1' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. pqg.c 423 V597 The compiler could delete the 'memset' function call, which is used to flush 'sha2' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. pqg.c 424 V597 The compiler could delete the 'memset' function call, which is used to flush 'U' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. pqg.c 471 V597 The compiler could delete the 'memset' function call, which is used to flush 'data' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. pqg.c 1208 V597 The compiler could delete the 'memset' function call, which is used to flush 'state' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. tlsprfalg.c 86 V597 The compiler could delete the 'memset' function call, which is used to flush 'outbuf' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. tlsprfalg.c 87 V597 The compiler could delete the 'memset' function call, which is used to flush 'newdeskey' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. pkcs11c.c 943 V597 The compiler could delete the 'memset' function call, which is used to flush 'randomData' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. pk11merge.c 298 V597 The compiler could delete the 'memset' function call, which is used to flush 'keyData' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. sslcon.c 2151 V597 The compiler could delete the 'memset' function call, which is used to flush 'randbuf' buffer. The RtlSecureZeroMemory () function should be used to erase the private data. keystuff.c 113 V610 Undefined behavior. Check the shift operator ' long ZEXPORT inflateMark (strm) z_streamp strm; { struct inflate_state FAR *state;
if (strm == Z_NULL || strm→state == Z_NULL) return -1L << 16; state = (struct inflate_state FAR *)strm->state; … } Согласно стандарту языка C++11, сдвиг отрицательного числа приводит к неопределённому поведению.Ещё похожее место:
V610 Undefined behavior. Check the shift operator '<<=. The left operand is negative ('cipher' = [-1..15]). strsclnt.c 1115 V555 The expression 'emLen — reservedLen — inputLen > 0' will work as 'emLen — reservedLen!= inputLen'. rsapkcs.c 708 #define PORT_Memset memset
static SECStatus eme_oaep_encode (unsigned char * em, unsigned int emLen, const unsigned char * input, unsigned int inputLen, HASH_HashType hashAlg, HASH_HashType maskHashAlg, const unsigned char * label, unsigned int labelLen, const unsigned char * seed, unsigned int seedLen) { … /* Step 2.b — Generate PS */ if (emLen — reservedLen — inputLen > 0) { PORT_Memset (em + 1 + (hash→length * 2), 0×00, emLen — reservedLen — inputLen); } … } Результатом разности беззнаковых чисел, кроме корректного и нулевого значений, также может быть невероятно большое число, получившееся в результате приведения отрицательного числа к беззнаковому типу. В этом фрагменте некорректное гигантское значение удовлетворит условию проверки, и функция 'memset' попытается обнулить огромный объём памяти.Впрочем, такое переполнение может вообще никогда не возникать — сложно судить, каковы предельные значения переменных, используемых в выражении. Но проверка в любом случае очень ненадёжная.
V677 Custom declaration of a standard 'BYTE' type. The system header file should be used: #include
typedef unsigned char BYTE; Напоследок — небольшое замечание по поводу объявления типов, которые уже определены в системных файлах.Аналогичные места:
V677 Custom declaration of a standard 'WORD' type. The system header file should be used: #include
Эта статья на английском Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. Analyzing the Network Security Services Library.
