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

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

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

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

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

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

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


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

>
>
>
Вновь проверяем Apache HTTP Server

Вновь проверяем Apache HTTP Server

06 Сен 2016

Проект Apache HTTP Server продолжает развиваться. Анализатор PVS-Studio не отстаёт и становится всё более мощным с каждой новой версией. Посмотрим, какие результаты покажет проверка на этот раз.

0427_Apache_HTTP_Recheck_ru/image1.png

Введение

Apache HTTP Server - это кроссплатформенный проект с открытым исходным кодом. Проект состоит из множества модулей. Ядро HTTP Server полностью разработано командой Apache Software Foundation на языке С. Прочие компоненты сервера созданы различными сторонними разработчиками, поддерживающими инициативу открытого программного обеспечения.

Ранние версии проекта Apache HTTP Server проверялись с помощью Сoverity. В проверенной версии не было обнаружено следов проверки другими анализаторами. Качество кода проекта на высоком уровне. Несмотря на это, анализатор PVS-Studio смог обнаружить несколько интересных ошибок.

Проект уже проверялся в 2011 году. О найденных в то время ошибках можно прочитать в статье "Лев Толстой и статический анализ кода".

Для проверки использовался анализатор PVS-Studio версии 6.08.

Неправильная проверка, что строка пустая

typedef struct {
  ....
  ap_regmatch_t *re_pmatch;
  apr_size_t re_nmatch;
  const char **re_source;
  ....
} ap_expr_eval_ctx_t;

static const char *ap_expr_eval_re_backref(
                     ap_expr_eval_ctx_t *ctx, ....)
{
  int len;

  if (!ctx->re_pmatch || 
      !ctx->re_source || 
      *ctx->re_source == '\0' ||    // <=
       ctx->re_nmatch < n + 1)
         return "";
....
}

Предупреждение:

V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: ** ctx->re_source == '\0'. util_expr_eval.c 199

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

Анализатор уже обнаруживал данную ошибку в проекте, о чём свидетельствует запись на странице с примерами ошибок, находимых диагностикой V528. Раз ошибка по-прежнему присутствует в проекте, о ней следует упомянуть вновь. Для исправления требуется изменить код следующим образом:

if (!ctx->re_pmatch  || 
    !ctx->re_source  || 
    !*ctx->re_source || 
    **ctx->re_source == '\0' ||
    ctx->re_nmatch < n + 1)
        return "";

Инкрементирование указателя вместо значения

apr_status_t iconv_uc_conv(...., apr_size_t *res)
{
  ....
  *res = (apr_size_t)(0);
  if (data == NULL) {
    *res = (apr_size_t) -1;
    return APR_EBADF;
  }
  ....
  if (size < 0) { 
     ....
     if (size)
       *res ++;                // <=
  }
  ....
}

Предупреждение:

V532 Consider inspecting the statement of '*pointer++' pattern. Probably meant: '(*pointer)++'. iconv_uc.c 114

В коде используется операция разыменования указателя. Полученное значение при этом не используется. Судя по коду функции, работать требовалось именно со значением указателя. А значит, для исправления ошибки надо повысить приоритет операции разыменования с помощью скобок: (*res) ++;

Неправильное затирание пароля

int get_password(struct passwd_ctx *ctx)
{
  ....
  if (strcmp(ctx->passwd, buf) != 0) {
      ctx->errstr = "password verification error";
      memset(ctx->passwd, '\0', strlen(ctx->passwd));
      memset(buf, '\0', sizeof(buf));
      return ERR_PWMISMATCH;
  }
  ....
  memset(buf, '\0', sizeof(buf));              // <=
  return 0;
  ....
}

Предупреждение:

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

Обязанность любой программы, работающей с конфиденциальными данными, затирать в памяти пароли и прочую важную информацию после завершения работы с ней. В этом фрагменте разработчики пытаются обнулить буфер, использовавшийся для хранения пароля. Выбранный ими способ должен был выполнить эту задачу. Однако для того, чтобы функция memset выполнила задачу правильно, буфер должен использоваться в последующем коде. Если же память заполняется нулями, а потом не используется, компилятор имеет право удалить функцию memset при построении кода. В результате информация, которая должна быть уничтожена, остаётся в памяти. Неизвестно, что будет с этим регионом памяти после, и куда попадёт эта неуничтоженная информация. Для очистки памяти надо использовать специализированные функции, такие как RtlSecureZeroMemory() или memset_s().

Хочется отметить, что это, пожалуй, самые серьезные из найденных дефектов для такого приложения как Apache HTTP Server!

Ещё несколько сообщений найденных этой диагностикой:

  • V597 The compiler could delete the 'memset' function call, which is used to flush 'x' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. apr_md4.c 362
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'tmpbuf' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. apr_md5.c 436
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'final' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. apr_md5.c 662

Неинициализированная переменная

static int warrsztoastr(...., const wchar_t * arrsz, int args)
{
  const apr_wchar_t *wch;
  apr_size_t totlen;
  apr_size_t newlen;
  apr_size_t wsize;
  сhar **env;
  char *pstrs;
  char *strs;
  int arg; 

  if (args < 0) {
    for (args = 1, wch = arrsz; wch[0] || wch[1]; ++wch)
      if (!*wch)
        ++args;
  }
  wsize = 1 + wch - arrsz; 

  newlen = totlen = wsize * 3 + 1;
  ....
  (void)apr_conv_ucs2_to_utf8(arrsz, &wsize, strs, &newlen);
  ....
  return args;
}

Предупреждение:

V614 Potentially uninitialized pointer 'wch' used. start.c 58

Функция подготавливает информацию для конвертирования строки из Wide Unicode в UTF-8. Если переменная args содержит отрицательное значение, значит количество символов в строке неизвестно, и их требуется посчитать.

Позднее, на основе адреса последнего символа в строке, содержащегося в переменной wch, и адреса начала строки arrsz, вычисляется значение wsize. С помощью wsize создаётся буфер для новой строки. Переменная wch инициализируется внутри цикла, который начинает работу только если args содержит отрицательное значение. Если же в функцию поступит положительное значение, то инициализации переменой не произойдёт. Это приведёт к неопределённому поведению, так как размер буфера будет вычислен некорректно.

На данный момент функция используется в проекте всего один раз с значением args равным -1. Поэтому эта ошибка долгое время оставалась бы незаметна, до тех пор, пока не понадобилось использовать функцию с положительным значением args. Я не могу знать, что планировалось делать внутри этой функции в такой ситуации. Как минимум странно видеть в виде параметра значение, которое в итоге возвращается как результат её работы. В то время как присутствие условного оператора в начале функции, в случае положительного значения args, делает её выполнение совершено бессмысленным.

Подозрительное выражение

static int is_quoted_pair(const char *s)
{
  int res = -1;
  int c;

  if (((s + 1) != NULL) && (*s == '\\')) {     // <=
    c = (int) *(s + 1);
    if (apr_isascii(c)) {
      res = 1;
    }
  }
  return (res);
}

Предупреждение:

V694 The condition ((s + 1) != ((void *) 0)) is only false if there is pointer overflow which is undefined behaviour anyway. mod_mime.c 531

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

Неверная проверка HRESULT

#define SHSTDAPI EXTERN_C DECLSPEC_IMPORT HRESULT STDAPICALLTYPE
SHSTDAPI SHGetMalloc(_Outptr_ IMalloc **ppMalloc);

LRESULT CALLBACK ConnectDlgProc(....)
{
  ....
  if (SHGetMalloc(&pMalloc)) {             // <=
   pMalloc->lpVtbl->Free(pMalloc, il);
   pMalloc->lpVtbl->Release(pMalloc);
  }
  ....
}

Предупреждение:

V545 Such conditional expression of 'if' operator is incorrect for the HRESULT type value 'SHGetMalloc(& pMalloc)'. The SUCCEEDED or FAILED macro should be used instead. apachemonitor.c 915

SHGetMalloc представляет из себя системную функцию и возвращает результат типа HRESULT. HRESULT 32-разрядное значение, логически состоящее из трёх полей. Его нельзя использовать как значение типа bool. Для обработки таких значений необходимо использовать макрос SUCCEEDED.

Лишняя операция?

static const char *process_resource_config_fnmatch(....)
{
  apr_status_t rv;
  ....
  rv = apr_dir_open(&dirp, path, ptemp);
  if (rv != APR_SUCCESS) {
    return apr_psprintf(p, 
               "Could not open config directory %s: %pm",
                path, &rv);
  }

  candidates = apr_array_make(ptemp, 1, sizeof(fnames));
  while (apr_dir_read(....) == APR_SUCCESS) {
     ....
     if (rest && (rv == APR_SUCCESS) &&              // <=
        (dirent.filetype != APR_DIR)) {     
          continue;
     }
     fnew = (fnames *) apr_array_push(candidates);
     fnew->fname = full_path;
  }
  ....
}

Предупреждение:

V560 A part of conditional expression is always true: (rv == 0). config.c 2029

Анализатор усмотрел лишнюю проверку внутри условия. На первый взгляд это просто избыточный код. При подробном рассмотрении видно, что такое значение переменной rv не позволило бы дойти до входа в цикл. К тому же странно, зачем использовать значение переменной, оставшееся после выполнения предыдущих операций, если в теле цикла она больше негде не используется.

Логично предположить, что перед условием также надо было использовать функцию rv = apr_dir_open(...); и тогда проверка результата rv обретает смысл. Конечно, могу ошибаться и это просто лишняя проверка, но на мой взгляд фрагмент кода обязательно надо проверить. Возможно, разработчики увидят, что в код цикла и обрабатываемых в нём операций действительно закралась ошибка и смогут вовремя её исправить.

Ещё несколько похожих ошибок:

  • V560 A part of conditional expression is always true: status == 0. mod_ident.c 217 (проект mod_ident)
  • V560 A part of conditional expression is always true: j == 0. mod_ident.c 217 (проект mod_ident)

Избыточное условие

static int uldap_connection_init(....)
{
  ....
  if (ldc->ChaseReferrals==AP_LDAP_CHASEREFERRALS_ON){
    if ((ldc->ReferralHopLimit != AP_LDAP_HOPLIMIT_UNSET) && 
         ldc->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON) {
          ....
    }
  }
  ....
}

Предупреждение:

V571 Recurring check. The 'ldc->ChaseReferrals == 1' condition was already verified in line 399. util_ldap.c 400

В этом примере приведено избыточное условие. Нет необходимости проверять одно и тоже выражение во внутреннем и внешнем условном операторе, так как условием входа во внутренний оператор, является прохождение проверок внешнего. В данном случае, весь код, заложенный внутри операторов, требует проверки всех выражений в обеих операторах if. Поэтому логичнее убрать внешний условный оператор. А для сохранения последовательности проверок предлагаю немного изменить выражение внутреннего условного оператора.

if (ldc->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON && 
   (ldc->ReferralHopLimit != AP_LDAP_HOPLIMIT_UNSET)) {
      ....
}

Неверная директива

#ifdef _MSC_VER
#pragma warning(disable: 4032)
#include <conio.h>
#pragma warning(default: 4032)
#else
#include <conio.h>
#endif

Предупреждение:

V665 Possibly, the usage of '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead. Check lines: 38, 40. apr_getpass.c 40

В приведённом фрагменте кода разработчики вместо возвращения директиве предыдущего значения используют значение по умолчанию для директивы. Это ошибочный подход. В таких случаях необходимо действовать иначе: запомнить предыдущее значение с помощью директивы #pragma warning(push), и вернуть его #pragma warning(pop). Исправленный код:

#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable: 4032)
#include <conio.h>
#pragma warning(pop)
#else
#include <conio.h>
#endif

Заключение

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

Популярные статьи по теме
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

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

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

Дата: 16 Окт 2017

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

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

Дата: 20 Мар 2017

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

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

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

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

Дата: 14 Апр 2016

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

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

Дата: 17 Янв 2019

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

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

Дата: 31 Май 2014

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

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

Дата: 21 Ноя 2018

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

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

Дата: 30 Янв 2019

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

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

Дата: 22 Окт 2018

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

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

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

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

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