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

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

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

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

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

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

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


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

>
>
>
По просьбе читателей, проверяем код LDA…

По просьбе читателей, проверяем код LDAP-сервера ReOpenLDAP

22 Авг 2016

В этой статье я хочу рассказать о проверке проекта ReOpenLDAP. Этот проект был реализован для решения проблем, возникших при эксплуатации OpenLDAP в инфраструктуре ПАО МегаФон - крупнейшего в России оператора мобильной связи. Сейчас ReOpenLDAP успешно работает в филиалах МегаФон по всей России, поэтому было бы интересно проверить столь высоконагруженный проект при помощи статического анализатора PVS-Studio.

0422_ReOpenLDAP_ru/image1.png

Введение

ReOpenLDAP, также известный как "TelcoLDAP" — это форк проекта OpenLDAP, созданный российскими разработчиками для применения в телекоммуникационной индустрии, с исправлением массы ошибок и работающей репликацией в мульти-мастер топологии. ReOpenLDAP является открытой реализацией сервера протокола LDAP на языке C.

ReOpenLDAP обеспечивает высокий уровень производительности:

  • Более 50 тысяч LDAP-изменений в секунду
  • Более 100 тысяч LDAP-запросов в секунду

Стоит отметить, что в наследство от OpenLDAP проект ReOpenLDAP получил 3185 операторов goto, которые в значительной мере затрудняют анализ, но даже несмотря на это было найдено некоторое количество ошибок.

Прошу записываться на тестирование Beta-версии PVS-Studio под Linux

Написание этой статьи стало возможным благодаря началу работ над созданием PVS-Studio for Linux. Именно в Linux мы проверяли проект ReOpenLDAP. Однако есть угроза, что Linux-версия закончит своё существование ещё до своего выхода в свет. Причина - мало практического интереса. Когда идёт какое-то обсуждение на форуме, то кажется, что самая большая проблема продукта PVS-Studio - это отсутствие версии для Linux. Но когда дело дошло до сбора контактов людей, готовых поучаствовать в тестировании Beta-версии, то оказалось, что их совсем мало. Примечание: мы писали о поиске энтузиастов в статье "PVS-Studio признаётся в любви к Linux".

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

0422_ReOpenLDAP_ru/image2.png

Просим вступить вас в ряды Beta-тестеров PVS-Studio для Linux. Это покажет, что к инструменту есть практический интерес. Ещё раз напишу, как можно вступить в ряды энтузиастов.

Если вы хотите помочь нам проверить работу PVS-Studio для Linux, прошу написать нам. Чтобы письма можно было проще обрабатывать, просим указать в теме письма строчку "PVS-Studio for Linux, Beta". Письма отправляйте по адресу support@viva64.com. Просим писать письма с корпоративных ящиков и кратко представиться. Мы будем благодарны всем, кто откликнется, но в первую очередь мы будем учитывать пожелания и рекомендации тех людей, которые потенциально со временем могут стать нашими клиентами.

Также прошу в письме дать ответы на следующие вопросы:

  • Под какой операционной системой планируется запускать анализатор?
  • Какую среду разработки вы используете?
  • Какой компилятор используется для сборки проекта?
  • Какую сборочную систему вы используете?

Когда появится версия, которую можно будет попробовать, мы напишем всем откликнувшимся письма. Заранее всем спасибо.

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

Ошибка в приоритете операций

Предупреждение PVS-Studio: V593 Consider reviewing the expression of the 'A = B == C' kind. The expression is calculated as following: 'A = (B == C)'. mdb_dump.c 150

static int dumpit(....)
{
  ....
  while ((rc = mdb_cursor_get(...) == MDB_SUCCESS)) {
    ....
  }
  ....
}

Автор ошибся с расположением закрывающейся круглой скобки в условии цикла while, что привело к ошибке в приоритете операций. В результате сначала выполняется сравнение, а затем результат этого сравнения записывается в переменную rc.

Данный код следует исправить так:

while ((rc = mdb_cursor_get(...)) == MDB_SUCCESS) {
  ....
}

Работа с нулевым указателем

Предупреждение PVS-Studio: V595 The 'key' pointer was utilized before it was verified against nullptr. Check lines: 1324, 1327. mdb.c 1324

char *
mdb_dkey(MDB_val *key, char *buf)
{
  ....
  unsigned char *c = key->mv_data; // <=
  ....
  if (!key)                        // <=
    return "";
  ....
}

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

Аналогичное предупреждение:

  • V595 The 'key' pointer was utilized before it was verified against nullptr. Check lines: 7282, 7291. mdb.c 7282

Подозрительный тернарный оператор

Предупреждение PVS-Studio: V583 The '?:' operator, regardless of its conditional expression, always returns one and the same value: "vlvResult". common.c 2119

static int
print_vlv(....)
{
  ....
  tool_write_ldif( ldif ? LDIF_PUT_COMMENT : LDIF_PUT_VALUE,
      ldif ? "vlvResult" : "vlvResult", buf, rc ); // <=
  }
  ....
}

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

....
tool_write_ldif( ldif ? LDIF_PUT_COMMENT : LDIF_PUT_VALUE,
      ldif ? "vlvResult: " : "vlvResult", buf, rc );
....

Вероятная опечатка в имени поля

Предупреждение PVS-Studio: V571 Recurring check. The 'if (s->state.r == 0)' condition was already verified in line 147. rurwl.c 148

void rurw_r_unlock(....) {
  ....
  if (s->state.r == 0) {  // <=
    if (s->state.r == 0)  // <=
      s->thr = 0;
    p->rurw_readers -= 1;
  }
  ....
}

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

void rurw_w_unlock(....) {
  ....
  if (s->state.w == 0) {
    if (s->state.r == 0)
      s->thr = 0;
    p->rurw_writer = 0;
  }
  ....
}

то можно предположить, что в одном из условий имелась в виду проверка s->state.w == 0. Однако, это лишь предположение, но в любом случае, разработчикам следует проверить этот участок кода, и либо скорректировать одно из условий, либо удалить дублирующую проверку.

Аналогичное место:

  • V571 Recurring check. The 'def->mrd_usage & 0x0100U' condition was already verified in line 319. mr.c 322

Перезапись параметра

Предупреждение PVS-Studio: V763 Parameter 'rc' is always rewritten in function body before being used. tls_o.c 426

static char *
tlso_session_errmsg(...., int rc, ....)
{
  char err[256] = "";
  const char *certerr=NULL;
  tlso_session *s = (tlso_session *)sess;
  rc = ERR_peek_error(); // <=
  ....
}

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

Неверный спецификатор форматирования

Предупреждение PVS-Studio: V576 Incorrect format. Consider checking the fourth actual argument of the 'snprintf' function. The SIGNED argument of memsize type is expected. conn.c 309

struct Connection {
  ....
  unsigned long c_connid;
  ....
}
....
static int
conn_create(....)
{
  ....
  bv.bv_len = snprintf( buf, sizeof( buf ),
                        "cn=Connection %ld", // <=
                        c->c_connid );
  ....
}

Спецификатор форматирования "%ld" не соответствует передаваемому в snprintf аргументу c->c_connid. Правильным спецификатором для unsigned long является "%lu". Использование "%ld" вместо "%lu" приведет к выводу неверных значений, при достаточно больших аргументах.

Аналогичные предупреждения:

  • V576 Incorrect format. Consider checking the third actual argument of the 'fprintf' function. The SIGNED integer type argument is expected. ure.c 1865
  • V576 Incorrect format. Consider checking the third actual argument of the 'fprintf' function. The SIGNED argument of memsize type is expected. tools.c 211
  • V576 Incorrect format. Consider checking the fourth actual argument of the 'fprintf' function. The UNSIGNED integer type argument is expected. mdb.c 1253

Неразыменованный указатель

Предупреждение PVS-Studio: V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *ludp->lud_filter != '\0'. backend.c 1525

int
fe_acl_group(....)
{
  ....
  if ( ludp->lud_filter != NULL &&
       ludp->lud_filter != '\0') // <=
  { 
    ....
  }
}

Автор кода хотел выявить ситуацию, когда указатель нулевой или строка пустая. Но забыл разыменовать указатель ludp->lud_filter, поэтому указатель просто дважды проверится на равенство NULL.

Следует разыменовать указатель:

  ....
  if ( ludp->lud_filter != NULL &&
       *ludp->lud_filter != '\0')
  ....

Аналогичные неразыменованные указатели:

  • V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *(* lsei)->lsei_values[0] == '\0'. syntax.c 240
  • V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *(* lsei)->lsei_values[1] != '\0'. syntax.c 241

Лишняя проверка

Предупреждение PVS-Studio: V560 A part of conditional expression is always true: !saveit. syncprov.c 1510

static void
syncprov_matchops( Operation *op, opcookie *opc, int saveit )
{
  ....
  if ( saveit || op->o_tag == LDAP_REQ_ADD ) {
    ....
  } else if ( op->o_tag == LDAP_REQ_MODRDN && !saveit ) {
    ....
  }
  ....
}

В ветке else нет смысла проверять saveit на равенство нулю, т.к. это уже было сделано в первом условии. Это ухудшает читабельность. И, возможно, это даже ошибка, так как предполагалось проверить что-то ещё.

Но скорее всего код достаточно упростить:

if ( saveit || op->o_tag == LDAP_REQ_ADD ) {
  ....
} else if ( op->o_tag == LDAP_REQ_MODRDN ) {
  ....
}

Опасное использование realloc

Предупреждение PVS-Studio: V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'lud.lud_exts' is lost. Consider assigning realloc() to a temporary pointer. ldapurl.c 306

int
main( int argc, char *argv[])
{
  ....
  lud.lud_exts = (char **)realloc( lud.lud_exts,
    sizeof( char * ) * ( nexts + 2 ) );
  ....
}

Выражение вида foo = realloc(foo, ....) является потенциально опасным. При невозможности выделения памяти realloc вернет нулевой указатель, перезаписав предыдущее значение указателя. Чтобы предотвратить это, рекомендуется сохранять значение указателя в дополнительной переменной перед использованием realloc.

Перезапись значения

Предупреждение PVS-Studio: V519 The 'ca.argv' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 7774, 7776. bconfig.c 7776

int
config_back_initialize( BackendInfo *bi )
{
  ....
  ca.argv = argv;      // <=
  argv[ 0 ] = "slapd";
  ca.argv = argv;      // <=
  ca.argc = 3;
  ca.fname = argv[0];
  ....
}

Если данный код корректен, то следует удалить первое лишнее присваивание.

Заключение

ReOpenLDAP является проектом, призванным обеспечить стабильную работу под высокой нагрузкой. Поэтому разработчики ответственно подходят к тестированию и используют специализированные инструменты такие как ThreadSanitizer и Valgrind. Однако, как мы видим, этого не всегда достаточно, так как при помощи PVS-Studio был выявлен ряд ошибок, хотя их и мало.

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

Предлагаю скачать и попробовать статический анализатор PVS-Studio на своем собственном проекте.

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

Дата: 21 Ноя 2018

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

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

Дата: 22 Дек 2018

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

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

Дата: 31 Май 2014

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

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

Дата: 30 Янв 2019

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

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

Дата: 16 Окт 2017

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

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

Дата: 19 Май 2017

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

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

Дата: 17 Янв 2019

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

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

Дата: 22 Окт 2018

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

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

Дата: 20 Мар 2017

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

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

Дата: 14 Апр 2016

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

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

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

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