Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Проверяем MatrixSSL с помощью PVS-Studi…

Проверяем MatrixSSL с помощью PVS-Studio и Cppcheck

02 Фев 2015

В статье я хочу рассказать о проверке проекта MatrixSSL статическими анализаторами C/C++ PVS-Studio и Cppcheck.

0304_MatrixSSL_PVS-Studio_ru/image1.png

Статья написана Павлом Пименовым, являющимся автором открытого peer-to-peer клиента FlylinkDC++. Статья публикуется в нашем блоге с его разрешения.

Порадовало наличие "из коробки" проекта для MS Visual Studio 2010.

Например, чтобы собрать openSSL из исходников в Visual C++ нужно немного поприседать с бубном :). Поэтому многие разработчики под windows используют готовые бинарные сборки openSSL, такие как Win32 OpenSSL Installation Project.

MatrixSSL - альтернативная библиотека алгоритмов шифрования распространяемым под лицензией GNU (также возможно получение коммерческой поддержки).

Исходный код open source версии можно получить на официальном сайте. Анализу подвергалась актуальная версия 3.7.1.

Об анализаторе

  • PVS-Studio - коммерческий статический анализатор, выявляющий ошибки в исходном коде приложений на языке C/C++/C++11. (использовалась версия PVS-Studio 5.21).
  • Cppcheck - бесплатный статический анализатор с открытым исходным кодом (использовалась версия Cppcheck 1.68).

Результаты проверки в PVS-Studio

Зачистка памяти

V512 A call of the 'memset' function will lead to underflow of the buffer 'ctx->pad'. hmac.c 136, 222, 356

...
// crypto\digest\digest.h
typedef struct {
#ifdef USE_SHA384
  unsigned char  pad[128];
#else
  unsigned char  pad[64];
#endif  

int32 psHmacMd5Final(psHmacContext_t *ctx, unsigned char *hash)
{ 
  memset(ctx->pad, 0x0, 64);
  return MD5_HASH_SIZE;
}
...

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

Думаю, здесь всё хорошо, но всё равно лучше для красоты затирать 64 или 128 байт. Можно написать так:

memset(ctx->pad, 0x0, sizeof(ctx->pad));

V597 The compiler could delete the 'memset' function call, which is used to flush 'tmp' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. aes.c 1139

...
int32 psAesEncrypt(psCipherContext_t *ctx, unsigned char *pt,
           unsigned char *ct, uint32 len)
{
  unsigned char  tmp[MAXBLOCKSIZE];
        .....
  memset(tmp, 0x0, sizeof(tmp));
  return len;
}
...

Оптимизатор выкидывает вызов стандартной функции memset(). Для библиотеки шифрования вероятно это критично и является потенциальной дырой.

Аналогичные проблемные места: aes.c 1139, aes.c 1190, aes.c 1191, des3.c 1564, des3.c 1609, des3.c 1610, corelib.c 304, pkcs.c 1625, pkcs.c 1680, pkcs.c 1741

V676 It is incorrect to compare the variable of BOOL type with TRUE. Correct expression is: 'QueryPerformanceFrequency(& hiresFreq) == FALSE'. osdep.c 52, 55

...
#define  PS_TRUE  1
#define  PS_FALSE   0  
int osdepTimeOpen(void)
{
  if (QueryPerformanceFrequency(&hiresFreq) != PS_TRUE) {
    return PS_FAILURE;
  }
  if (QueryPerformanceCounter(&hiresStart) != PS_TRUE) {
    return PS_FAILURE;
  }
...

PS_TRUE объявлена как "1". В MSDN про возврат функции QueryPerformanceFrequency написано "If the installed hardware supports a high-resolution performance counter, the return value is nonzero" Т.е. надежнее написать QueryPerformanceCounter() == PS_FALSE

V547 Expression '(id = ssl->sessionId) == ((void *) 0)' is always false. Pointer 'id = ssl->sessionId' != NULL. matrixssl.c 2061

...
typedef struct ssl {
        ...
  unsigned char  sessionIdLen;
  unsigned char  sessionId[SSL_MAX_SESSION_ID_SIZE];

int32 matrixUpdateSession(ssl_t *ssl)
{
#ifndef USE_PKCS11_TLS_ALGS
  unsigned char  *id;
  uint32  i;

  if (!(ssl->flags & SSL_FLAGS_SERVER)) {
    return PS_ARG_FAIL;
  }
  if ((id = ssl->sessionId) == NULL) {
    return PS_ARG_FAIL;
  }
...

Тут явная ошибка: условие никогда не выполнится так как sessionId объявлен как массив из 32 байт и у него не может быть адреса равного NULL. Ошибка конечно не серьезная. Пожалуй, это можно назвать и просто лишней бессмысленной проверкой.

V560 A part of conditional expression is always true: 0x00000002. osdep.c 265

...
#define FILE_SHARE_READ                 0x00000001  
#define FILE_SHARE_WRITE                0x00000002  

  if ((hFile = CreateFileA(fileName, GENERIC_READ,
      FILE_SHARE_READ && FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
      FILE_ATTRIBUTE_NORMAL, NULL)) == INVALID_HANDLE_VALUE) {
    psTraceStrCore("Unable to open %s\n", (char*)fileName);
        return PS_PLATFORM_FAIL;
...

Опечатка: вместо FILE_SHARE_READ | FILE_SHARE_WRITE записали && получилось 1 && 2 == 1

что эквивалентно одному FILE_SHARE_READ.

Возможно ошибочное условие

V590 Consider inspecting the '* c != 0 && * c == 1' expression. The expression is excessive or contains a misprint. ssldecode.c 3539


...
    if (*c != 0 && *c == 1) {
#ifdef USE_ZLIB_COMPRESSION
      ssl->inflate.zalloc = NULL;
...

Возможная просадка производительности

V814 Decreased performance. The 'strlen' function was called multiple times inside the body of a loop. x509.c 226

...
  memset(current, 0x0, sizeof(psList_t));
  chFileBuf = (char*)fileBuf;
  while (fileBufLen > 0) {
  if (((start = strstr(chFileBuf, "-----BEGIN")) != NULL) &&
...
      start += strlen("CERTIFICATE-----");
      if (current == NULL) {
...

Тут анализатор внутри цикла while() обнаружил вызов strlen() для параметра который не меняется, в общем случае это не оптимально, но в данном конкретном т.к. на вход strlen() передается константа известная на этапе компиляции, то оптимизатор в режиме /O2 вообще уберет вызов функции и подставит значение константы вычисленное на этапе компиляции.

Результаты проверки в Cppcheck

Этот анализатор показал меньше предупреждений, но есть те которые PVS-Studio не смог детектировать.

Все они на работу библиотеки не влияют так как лежат в юнит-тестах относятся в crypto\test.

"Контрольный return в голову"

Consecutive return, break, continue, goto or throw statements are unnecessary. The second statement can never be executed, and so should be removed.

...

int32 psSha224Test(void)
{
  runDigestTime(&ctx, HUGE_CHUNKS, SHA224_ALG);
  
     return PS_SUCCESS;
  return PS_SUCCESS;
}
...

Ошибка copy-paste. В конце две одинаковых строчки: return PS_SUCCESS;.

Аналогичная опечатка находится в функции psSha384Test(void).

Утечка памяти

Memory leak: table

Неопасная в данном случае ошибка, но приятно что Cppcheck ее ловит код находится в файлах и выглядит так (copy-paste):

  • crypto\test\eccperf\eccperf.c
  • crypto\test\rsaperf\rsaperf.c
...
  table = malloc(tsize * sizeof(uint32));  
  if ((sfd = fopen("perfstat.txt", "w")) == NULL) {
    return PS_FAILURE;
  }
...

Ресурсы лучше заказывать перед тем когда они действительно необходимы. Если посмотреть код в этих файлах, то окажется что table вообще не используется, т.е. тут вызов функции malloc() и в конце функции free(table) лишние.

Заключение

Я явлюсь разработчиком FlylinkDC++ и уже более двух лет использую анализатор PVS-Studio, который нам подарили как Open Source проекту. Анализатор неоднократно помогал локализовать различные ошибки, как в своем коде, так и в коде сторонних библиотек. Благодаря регулярным проверкам код FlylinkDC++ стал немного надёжней и безопасней. И это замечательно.

Популярные статьи по теме
Как различить C и C++ разработчиков по их коду

Дата: 12 Май 2022

Автор: Гость

Так уж случилось, что я пишу код для разных IoT-железок, связанных с электричеством, типа зарядных станций автомобилей. Поскольку аппаратных ресурсов, как правило, вполне достаточно, то основным фоку…
Отладочный вывод на микроконтроллерах: как Concepts и Ranges отправили мой printf на покой

Дата: 06 Май 2022

Автор: Гость

Здравствуйте! Меня зовут Александр, и я работаю программистом микроконтроллеров.
Нереальный baselining или доработки PVS-Studio для Unreal Engine проектов

Дата: 26 Апр 2022

Автор: Валерий Комаров

Статический анализатор PVS-Studio постоянно развивается: улучшаются различные механизмы, происходит интеграция с игровыми движками, IDE, CI/CD и другими системами и сервисами. Благодаря этому несколь…
Разбор некоторых вредных советов для С++ программиста

Дата: 21 Апр 2022

Автор: Андрей Карпов

Юмор юмором, но осторожность не повредит. Вдруг кому-то не до конца понятно, почему какой-то из советов является вредным. Здесь можно найти соответствующие пояснения.
Четыре причины проверять, что вернула функция malloc

Дата: 20 Апр 2022

Автор: Андрей Карпов

Некоторые разработчики пренебрежительно относятся к проверкам: удалось ли выделить память с помощью функции malloc или нет. Их логика проста – памяти всегда должно хватить. А если не хватит, всё равн…

Комментарии (0)

Следующие комментарии
Этот сайт использует куки и другие технологии, чтобы предоставить вам более персонализированный опыт. Продолжая просмотр страниц нашего веб-сайта, вы принимаете условия использования этих файлов. Если вы не хотите, чтобы ваши данные обрабатывались, пожалуйста, покиньте данный сайт. Подробнее →
Принять