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

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

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

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

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

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

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


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

>
>
>
Немного про OpenSSL

Немного про OpenSSL

17 Дек 2012

Небольшая заметка о результатах проверки проекта OpenSSL с помощью анализатора PVS-Studio. Я анализировал версию openssl-0.9.8-stable-SNAP-20121208.

О проверке OpenSSL

Недавно я написал заметку "Безопасность, безопасность! А вы её тестируете?" о проверке проекта TOR. За компанию я упомянул библиотеку OpenSSL, которая входит в проект TOR.

Статья активно обсуждалась на некоторых программистских ресурсах. Оказалось, что программистов весьма заботит качество библиотеки OpenSSL. Однако, я не смог ответить на некоторые вопросы касающиеся этой библиотеки. Также меня обвинили в том, что я не уведомил разработчиков библиотеки OpenSSL обо всех подозрительных местах.

Хочу пояснить эту ситуацию. Дело в том, что изучая TOR, я не планировал проверять библиотеку OpenSSL и изучать результаты её анализа. Эта библиотека просто случайно попалась под руку. В проект TOR была включена, какая то версия OpenSSL. Она и была обработана анализатором PVS-Studio за компанию.

Теперь я решил исправить ситуацию и специально скачал и проверил свежую версию библиотек OpenSSL.

Если честно, рассказывать мне особенно нечего. Мне не удалось обнаружить почти ничего подозрительного. Те ошибки, которые я описывал в статье, уже исправлены. OpenSSL – это качественный проект. Библиотека уже проверена множеством инструментов (Clang, Cppcheck, Coverity, DoubleCheck, Coccinelle, Klocwork и другими). В общем, библиотека хорошо вылизана. И найти в ней хотя бы одну ошибку, это будет уже подвиг.

Итак, расскажу, что подозрительного я нашел в коде библиотеки OpenSSL. Скорее всего, это несущественные недочеты, а не ошибки. Но надо хоть про что-то написать. :)

Подозрительный фрагмент N1

EVP_PKEY *STORE_get_private_key(....)
{
  STORE_OBJECT *object;
  ....
  if (!object || !object->data.key || !object->data.key)
  {
    STOREerr(STORE_F_STORE_GET_PRIVATE_KEY,
             STORE_R_FAILED_GETTING_KEY);
    return 0;
  }
  ....
}

Диагностическое сообщение PVS-Studio: V501 There are identical sub-expressions '!object->data.key' to the left and to the right of the '||' operator. str_lib.c 475

Два раза повторяется условие "!object->data.key". Скорее всего, вторая проверка просто лишняя и ничего страшного тут нет. Если вдруг окажется, что здесь хотели проверить другой член класса, тогда это конечно беда.

Эту странную проверку можно встретить ещё в трех местах файла str_lib.c (видимо это Copy-Paste):

  • V501 There are identical sub-expressions '!object->data.key' to the left and to the right of the '||' operator. str_lib.c 616
  • V501 There are identical sub-expressions '!object->data.key' to the left and to the right of the '||' operator. str_lib.c 670
  • V501 There are identical sub-expressions '!object->data.key' to the left and to the right of the '||' operator. str_lib.c 811

Подозрительный фрагмент N2

Есть несколько мест, где указатель в начале используется, а затем проверяется на нулевое значение. Но достаточно подозрительным мне показался только один фрагмент:

int OBJ_obj2txt(char *buf, int buf_len,
                const ASN1_OBJECT *a, int no_name)
{
  ....
  if ((a == NULL) || (a->data == NULL)) {
    buf[0]='\0';
    return(0);
  }
  ....
  if (buf)
  ....
}

Диагностическое сообщение PVS-Studio: V595 The 'buf' pointer was utilized before it was verified against nullptr. Check lines: 448, 461. obj_dat.c 448

В начале проверяется, что 'a' или 'a->data' равны нулю. Если это так, то используется указатель 'buf'. Но ведь и указатель 'buf' тоже может быть равен нулю. На это намекает проверка '"if (buf)", расположенная чуть ниже в коде.

Подозрительный фрагмент N3

В следующем фрагменте кода, анализатору PVS-Studio кажется всё-таки удалось найти настоящую полновесную ошибку.

int ssl3_get_cert_verify(SSL *s)
{
  int type=0,i,j;
  ....
  if ((peer != NULL) && (type | EVP_PKT_SIGN))
  {
    al=SSL_AD_UNEXPECTED_MESSAGE;
    SSLerr(SSL_F_SSL3_GET_CERT_VERIFY,
           SSL_R_MISSING_VERIFY_MESSAGE);
    goto f_err;
  }
  ....
}

Диагностическое сообщение PVS-Studio: V617 Consider inspecting the condition. The '0x0010' argument of the '|' bitwise operation contains a non-zero value. s3_srvr.c 2394

Выражение "(type | EVP_PKT_SIGN)" всегда истинно. Возможно, здесь должно было быть написано: "type & EVP_PKT_SIGN".

Подозрительный фрагмент N4

Есть несколько бессмысленных проверок следующего вида:

int MAIN(int argc, char **argv)
{
  ....
  long dsa_c[DSA_NUM][2];
  ....
  if (dsa_c[i] == 0)
  {
    dsa_c[i][0]=1;
    dsa_c[i][1]=1;
  }
  ....
}

Диагностическое сообщение PVS-Studio: V600 Consider inspecting the condition. The 'dsa_c[i]' pointer is always not equal to NULL. speed.c 1486

Здесь 'dsa_c' это двумерный массив. Поэтому выражение "dsa_c[i] == 0" всегда истинно и не имеет смысла. Рядом есть код следующего вида:

if (rsa_c[i][0] == 0)
{
  rsa_c[i][0]=1;
  rsa_c[i][1]=20;
}

Возможно и с массивом 'dsa_c' следует работать таким же образом. Тогда код должен выглядеть так:

if (dsa_c[i][0] == 0)
{
  dsa_c[i][0]=1;
  dsa_c[i][1]=1;
}

Такая странная проверка есть ещё в нескольких местах:

  • V600 Consider inspecting the condition. The 'ecdsa_c[i]' pointer is always not equal to NULL. speed.c 1506
  • V600 Consider inspecting the condition. The 'ecdsa_c[i]' pointer is always not equal to NULL. speed.c 1523
  • V600 Consider inspecting the condition. The 'ecdsa_c[i]' pointer is always not equal to NULL. speed.c 1540
  • V600 Consider inspecting the condition. The 'ecdh_c[i]' pointer is always not equal to NULL. speed.c 1560
  • V600 Consider inspecting the condition. The 'ecdh_c[i]' pointer is always not equal to NULL. speed.c 1577
  • V600 Consider inspecting the condition. The 'ecdh_c[i]' pointer is always not equal to NULL. speed.c 1594

Неподозрительный момент

Есть ряд мелких недочетов. Это точно не ошибки. Просто лишний код. Вот пример избыточности в условии:

int ASN1_PRINTABLE_type(const unsigned char *s, int len)
{
  int c;
  ....
  c= *(s++);
    if (!(  ((c >= 'a') && (c <= 'z')) ||
      ((c >= 'A') && (c <= 'Z')) ||
      (c == ' ') ||
      ((c >= '0') && (c <= '9')) ||
      (c == ' ') || (c == '\'') ||
      (c == '(') || (c == ')') ||
      (c == '+') || (c == ',') ||
      (c == '-') || (c == '.') ||
      (c == '/') || (c == ':') ||
      (c == '=') || (c == '?')))
      ia5=1;
  ....
}

Диагностическое сообщение PVS-Studio: V501 There are identical sub-expressions '(c == ' ')' to the left and to the right of the '||' operator. a_print.c 76

То, что символ является пробелом, повторяется два раза. Просто по невнимательности проверок написали два раза. Ничего страшного. Видел ещё подобные места, но писать про них не интересно.

Последнее место, которое заслуживает внимания

Я долго не мог понять, в некоторых местах анализатор нашел приведение memsize-типа к 32-битному типу, а затем обратно к memsize-типу. Вот одно их таких мест:

int ec_GFp_simple_points_make_affine(const EC_GROUP *group,
  size_t num, EC_POINT *points[], BN_CTX *ctx)
{
  BIGNUM **heap = NULL;
  size_t pow2 = 0;
  ....
  heap = OPENSSL_malloc(pow2 * sizeof heap[0]);
  ....
}

Диагностическое сообщение PVS-Studio: V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: 'pow2'. ecp_smpl.c 1576

Оказывается, макрос OPENSSL_malloc написан некрасиво.

void *CRYPTO_malloc(int num, const char *file, int line);
#define OPENSSL_malloc(num) CRYPTO_malloc((int)num,__FILE__,__LINE__)

В результате, после препроцессирования получается следующий код:

heap = CRYPTO_malloc ((int)pow2 * sizeof heap[0], __FILE__,__LINE__);

В результате начинается бестолковые приведения типов. В начале переменная 'pow2' типа size_t явно приводится к типу 'int'. Затем при умножении на 'sizeof()' тип выражения становится опять size_t. После этого при вызове функции CRYPTO_malloc() вновь происходит приведение к типу 'int'. На этот раз приведение уже неявное.

В общем, от этого приведения типа, нет никакого толку. Только потенциальное повод допустить ошибку. Например, можно написать что-то такое:

int *p1, *p2;
int x, y;
....
p = OPENSSL_malloc(p1 == p2 ? x : y);

На 64-битной системе указатель 'p1' потеряет старшие биты, и результат сравнения может дать неверный результат.

Это конечно выдуманный пример, но всё равно не хорошо создавать такие макросы. Как минимум, следует переписать макрос так:

#define OPENSSL_malloc(num) CRYPTO_malloc((int)(num),
                                           __FILE__,__LINE__)

А ещё лучше вообще не использовать здесь тип 'int'. Размер выделяемой памяти должен передаваться в переменной memsize-типа. Например, это тип 'size_t'.

Заключение

Спасибо всем за внимание. Буду рад, если эта заметка поможет как-то улучшить библиотеку OpenSSL. Как всегда прошу авторов не ограничиваться подозрительными местами, описанными здесь. Лучше ещё раз проверить библиотеку и самостоятельно изучить отчёт. Мы бесплатно предоставляем разработчикам open-source библиотек ключ на некоторое время. Возможно, удастся заметить опасные места, которым я не придал значения.

Последние статьи:

Опрос:

Популярные статьи по теме
Holy C++

Дата: 23 Ноя 2022

Автор: Гость

В этой статье постараюсь затронуть все вещи, которые можно без зазрения совести выкинуть из С++, не потеряв ничего (кроме боли), уменьшить стандарт, нагрузку на создателей компиляторов, студентов, из…
Продление жизни временных значений в С++: рецепты и подводные камни

Дата: 01 Ноя 2022

Автор: Гость

Прочитав эту статью, вы узнаете следующее: способы, которыми можно продлить время жизни временного объекта в С++; рекомендации и подводные камни этого механизма, с которыми может столкнуться С++ прог…
Как мы баг в PVS-Studio искали или 278 Гигабайтов логов

Дата: 28 Окт 2022

Автор: Григорий Семенчев, Сергей Ларин, Филипп Хандельянц

Предлагаем вашему вниманию интересную историю о поиске бага внутри анализатора PVS-Studio. Да, мы тоже допускаем ошибки, но мы готовы засучить рукава и залезть в самую глубину "кроличьей норы".
0, 1, 2, Фредди забрал Blender

Дата: 26 Окт 2022

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

Эта статья могла бы получить название "Как PVS-Studio защищает от поспешных правок кода, пример N7". Однако так именовать статьи становится скучновато. Поэтому сейчас вы узнаете, причём здесь Фредди …
Примеры ошибок, которые может обнаружить PVS-Studio в коде LLVM 15.0

Дата: 25 Окт 2022

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

Компиляторы развиваются и выдают всё больше предупреждений. Остаются ли преимущества от использования статических анализаторов кода, таких как PVS-Studio? Да, так как анализаторы тоже развиваются. Пе…

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

Следующие комментарии
Unicorn with delicious cookie
Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо