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

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

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

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

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

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

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


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

>
>
>
Побочный результат: проверяем Firebird …

Побочный результат: проверяем Firebird с помощью PVS-Studio

21 Фев 2014

Сейчас мы заняты большой задачей. Мы хотим провести сравнение анализаторов кода: Cppcheck, PVS-Studio и Visual Studio 2013 (встроенный анализатор кода). Для этого мы решили проверить не менее 10 открытых проектов и проанализировать отчёты, которые выдадут анализаторы. Это очень трудоёмкая задача и пока она не завершена. Но так как ряд проектов уже проверен, то про некоторые из них можно написать статьи. Чем я сейчас и займусь. Для начала опишу, что интересного удалось найти с помощью PVS-Studio в Firebird.

0235_Firebird_ru/image1.png

Firebird

Firebird (FirebirdSQL) — компактная, кроссплатформенная, свободная система управления базами данных (СУБД), работающая на Linux, Microsoft Windows и разнообразных Unix платформах.

Сайт: http://www.firebirdsql.org/

Описание в Wikipedia: Firebird

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

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

static const UCHAR* compile(const UCHAR* sdl, sdl_arg* arg)
{
  SLONG n, count, variable, value, sdl_operator;
  ....
  switch (op)
  {
    ....
    case isc_sdl_add:
      sdl_operator = op_add;
    case isc_sdl_subtract:
      if (!sdl_operator)
        sdl_operator = op_subtract;
  ......
}

V614 Uninitialized variable 'sdl_operator' used. sdl.cpp 404

Мне кажется, оператор 'break' между "case isc_sdl_add:" и "case isc_sdl_subtract:" отсутствует специально. Здесь не учтён случай, когда мы сразу попадаем в "case isc_sdl_subtract:". Если это произойдёт, то переменная 'sdl_operator' ещё не будет инициализирована.

Другая схожая ситуация. Переменная 'fieldNode' может остаться неинициализированной, если "field == false".

void blb::move(....)
{
  ....
  const FieldNode* fieldNode;
  if (field)
  {
    if ((fieldNode = ExprNode::as<FieldNode>(field)))
    ....
  }
  ....
  const USHORT id = fieldNode->fieldId;
  ....
}

V614 Potentially uninitialized pointer 'fieldNode' used. blb.cpp 1043

Вот почему плохо в функции давать разным переменным одно и то же имя:

void realign(....)
{
  for (....)
  {
    UCHAR* p = buffer + field->fld_offset;
    ....
    for (const burp_fld* field = relation->rel_fields;
         field; field = field->fld_next)
    {
      ....
      UCHAR* p = buffer + FB_ALIGN(p - buffer, sizeof(SSHORT));
  ........
}

V573 Uninitialized variable 'p' was used. The variable was used to initialize itself. restore.cpp 17535

При инициализации второй переменной 'p' хотели использовать значение первой переменной 'p'. А получилось, что используется вторая, ещё неинициализированная переменная.

Для авторов проекта. Посмотрите ещё сюда: restore.cpp 17536

Опасное сравнение строк (уязвимость)

Обратите внимание, что результат работы функции memcmp() помещается в переменную типа 'SSHORT'. 'SSHORT' это не что иное, как синоним типа 'short'.

SSHORT TextType::compare(
  ULONG len1, const UCHAR* str1, ULONG len2, const UCHAR* str2)
{
  ....
  SSHORT cmp = memcmp(str1, str2, MIN(len1, len2));

  if (cmp == 0)
    cmp = (len1 < len2 ? -1 : (len1 > len2 ? 1 : 0));

  return cmp;
}

V642 Saving the 'memcmp' function result inside the 'short' type variable is inappropriate. The significant bits could be lost breaking the program's logic. texttype.cpp 338

Вы недоумеваете, что здесь не так?

Давайте вспомним, что функция memcmp() возвращает значение типа 'int'. Но результат помещается в переменную типа 'short'. Происходит потеря значений старших бит. Это опасно!

Функция возвращает следующие значения: меньше нуля, ноль или больше нуля. Под "больше нуля" может подразумеваться что угодно. Это может быть 1, 2 или 19472341. Нельзя сохранять результат работы функции memcmp() в переменную, размером меньше чем размер типа 'int'.

Проблема может показаться надуманной. Но это самая настоящая проблема уязвимости. Именно уязвимостью была признана аналогичная ошибка в коде MySQL: Security vulnerability in MySQL/MariaDB sql/password.c. Там результат помещался в переменную типа 'char'. Тип 'short' с точки зрения безопасности не сильно лучше.

Аналогичные опасные сравнения можно найти здесь:

  • cvt2.cpp 256
  • cvt2.cpp 522

Опечатки

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

int Parser::parseAux()
{
  ....
  if (yyps->errflag != yyps->errflag) goto yyerrlab;
  ....
}

V501 There are identical sub-expressions to the left and to the right of the '!=' operator: yyps->errflag != yyps->errflag parse.cpp 23523

Думаю, комментарии здесь не нужны. А вот здесь, наверное, использовался Copy-Paste:

bool CMP_node_match( const qli_nod* node1, const qli_nod* node2)
{
  ....
  if (node1->nod_desc.dsc_dtype != node2->nod_desc.dsc_dtype ||
      node2->nod_desc.dsc_scale != node2->nod_desc.dsc_scale ||
      node2->nod_desc.dsc_length != node2->nod_desc.dsc_length)
  ....
}

V501 There are identical sub-expressions 'node2->nod_desc.dsc_scale' to the left and to the right of the '!=' operator. compile.cpp 156

V501 There are identical sub-expressions 'node2->nod_desc.dsc_length' to the left and to the right of the '!=' operator. compile.cpp 157

Получается, что в функции CMP_node_match() неправильно сравниваются члены класса 'nod_desc.dsc_scale' и 'nod_desc.dsc_length'.

Ещё одну опечатку авторы проекта могут посмотреть здесь: compile.cpp 183

Странные циклы

static processing_state add_row(TEXT* tabname)
{
  ....
  unsigned i = n_cols;
  while (--i >= 0)
  {
    if (colnumber[i] == ~0u)
  {
       bldr->remove(fbStatus, i);
       if (ISQL_errmsg(fbStatus))
         return (SKIP);
    }
  }
  msg.assignRefNoIncr(bldr->getMetadata(fbStatus));
  ....
}

V547 Expression '-- i >= 0' is always true. Unsigned type value is always >= 0. isql.cpp 3421

Переменная 'i' имеет тип 'unsigned'. Это значит, что переменная 'i' всегда больше или равно 0. В результате условие (--i >= 0) не имеет смысла. Оно всегда истинно.

Этот цикл наоборот закончится раньше, чем нужно:

SLONG LockManager::queryData(....)
{
  ....
  for (const srq* lock_srq = (SRQ) 
         SRQ_ABS_PTR(data_header.srq_backward);
     lock_srq != &data_header;
     lock_srq = (SRQ) SRQ_ABS_PTR(lock_srq->srq_backward))
  {
    const lbl* const lock = ....;
    CHECK(lock->lbl_series == series);
    data = lock->lbl_data;
    break;
  }
  ....
}

Что это за подозрительный 'break' ?

Ещё одна аналогичная ситуация здесь: pag.cpp 217

Классика

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

int CCH_down_grade_dbb(void* ast_object)
{
  ....
  SyncLockGuard bcbSync(
    &bcb->bcb_syncObject, SYNC_EXCLUSIVE, "CCH_down_grade_dbb");
  bcb->bcb_flags &= ~BCB_exclusive;

  if (bcb && bcb->bcb_count)
  ....
}

V595 The 'bcb' pointer was utilized before it was verified against nullptr. Check lines: 271, 274. cch.cpp 271

В начале, указатель 'bcb' разыменовывается в выражении "bcb->bcb_flags &= ....". Из следующей проверки видно, что 'bcb' может оказаться равен нулю.

Список таких мест (31 предупреждение): firebird-V595.txt

Shift operators

Так как Firebird собирается разными компиляторами под разные платформы, то стоит поправить сдвиги, которые приводят к неопределённому поведению. Они вполне могут проявить себя со временем негативным образом.

const ULONG END_BUCKET = (~0) << 1;

V610 Undefined behavior. Check the shift operator '<<. The left operand '(~0)' is negative. ods.h 337

Нельзя сдвигать отрицательные числа. Подробнее: "Не зная брода, не лезь в воду. Часть третья".

Здесь лучше сделать так:

const ULONG END_BUCKET = (~0u) << 1;

И ещё два плохих сдвига:

  • exprnodes.cpp 6185
  • array.cpp 845

Бессмысленные проверки

static processing_state add_row(TEXT* tabname)
{
  ....
  unsigned varLength, scale;
  ....
  scale = msg->getScale(fbStatus, i);
  ....
  if (scale < 0)
  ....
}

V547 Expression 'scale < 0' is always false. Unsigned type value is never < 0. isql.cpp 3716

Переменная 'scale' имеет тип 'unsigned'. Сравнение (scale < 0) не имеет смысла.

Аналогично: isql.cpp 4437

Посмотрим на другую функцию:

static bool get_switches(....)
  ....
  if (**argv != 'n' || **argv != 'N')
  {
    fprintf(stderr, "-sqlda :  "
            "Deprecated Feature: you must use XSQLDA\n ");
    print_switches();
    return false;
  }
  ....
}

Неправильно обрабатываются аргументы командной строки. Условие (**argv != 'n' || **argv != 'N') выполняется всегда.

Разное

void FB_CARG Why::UtlInterface::getPerfCounters(
  ...., ISC_INT64* counters)
{
  unsigned n = 0;
  ....
  memset(counters, 0, n * sizeof(ISC_INT64));
  ....
}

V575 The 'memset' function processes '0' elements. Inspect the third argument. perf.cpp 487

Кажется, что переменной 'n' в теле функции забыли присвоить значение, отличное от нуля.

Функция convert() в качестве третьего аргумента принимает длину строки:

ULONG convert(const ULONG srcLen,
              const UCHAR* src,
              const ULONG dstLen,
              UCHAR* dst,
              ULONG* badInputPos = NULL,
              bool ignoreTrailingSpaces = false);

Однако, используется функция неправильно:

string IntlUtil::escapeAttribute(....)
{
  ....
  ULONG l;
  UCHAR* uc = (UCHAR*)(&l);
  const ULONG uSize =
    cs->getConvToUnicode().convert(size, p, sizeof(uc), uc);
  ....
}

V579 The convert function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. intlutil.cpp 668

Мы имеем дело с 64-битной ошибкой, которая проявит себя в Win64.

Выражение 'sizeof(uc)' возвращает размер указателя, а не размер буфера. Это не страшно, если размер указателя совпадает с размером переменной типа 'unsigned long'. Размер указателя совпадает с размером типа 'long', если мы программируем для Linux. Не будет проблем и в Win32.

Ошибка возникает, когда мы скомпилируем приложение для Win64. Функция convert() будет думать, что размер буфера равен 8 байт (размер указателя). А на самом деле, размер буфера равен 4 байтам.

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

Заключение

Читателю, наверное, интересно узнать, что удалось найти в этом проекте с помощью Cppcheck и VS2013. Да, эти анализаторы нашли кое-что, что не удалось PVS-Studio. Но совсем немного. На этом проекте PVS-Studio показал себя лидером. Более подробную информацию можно будет найти в статье о сравнении, которую мы уже скоро начнём писать.

Популярные статьи по теме
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

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

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

Дата: 30 Янв 2019

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

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

Дата: 19 Май 2017

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

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

Дата: 21 Ноя 2018

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

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

Дата: 17 Янв 2019

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

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

Дата: 14 Апр 2016

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

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

Дата: 16 Окт 2017

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

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

Дата: 31 Май 2014

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

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

Дата: 22 Дек 2018

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

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

Дата: 27 Июн 2017

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

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

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

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

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