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

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

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

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

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

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Ваше сообщение отправлено.

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


Если вы так и не получили ответ, пожалуйста, проверьте папку
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 библиотек ключ на некоторое время. Возможно, удастся заметить опасные места, которым я не придал значения.

Популярные статьи по теме
Эффект последней строки

Дата: 31 Май 2014

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

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
PVS-Studio для Java

Дата: 17 Янв 2019

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

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

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

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

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

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

Дата: 22 Дек 2018

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

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

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

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

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

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
PVS-Studio ROI

Дата: 30 Янв 2019

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

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Дата: 22 Окт 2018

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

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь ок…
Любите статический анализ кода!

Дата: 16 Окт 2017

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

Я в шоке от возможностей статического анализа кода, хотя сам участвую в разработке инструмента PVS-Studio. На днях я был искренне удивлён тому, что анализатор оказался умнее и внимательнее меня.

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

Следующие комментарии

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