Предновогодняя проверка PostgreSQL

в 6:06, , рубрики: c++, code review, memcmp, postgresql, pvs-studio, Блог компании PVS-Studio, статический анализ кода, метки: , , , , ,

PVS-Studio, PostgreSQL
Год заканчивается, а я давно не писал заметок о проверке открытых проектов. Мне уже неоднократно предлагали проверить проект PostgreSQL Database Management System. Этим я и занялся. К сожалению, грандиозной и интересной статьи не получится. Я заметил только несколько типовых ошибок. Так что в этот раз получилась совсем небольшая статья.

PostgreSQL — свободная объектно-реляционная система управления базами данных (СУБД). PostgreSQL базируется на языке SQL и поддерживает многие из возможностей стандарта SQL:2003 (ISO/IEC 9075). Подробнее о проекте можно почитать на сайте Wikipedia и на самом сайте проекта.

1. Опечатки при использовании функции memcmp()

Datum pg_stat_get_activity(PG_FUNCTION_ARGS)
{
  ....
  if (memcmp(&(beentry->st_clientaddr), &zero_clientaddr,
             sizeof(zero_clientaddr) == 0))
  ....
}

Предупреждение выданное PVS-Studio: V526 The 'memcmp' function returns 0 if corresponding buffers are equal. Consider examining the condition for mistakes. postgres pgstatfuncs.c 712

Также на эту строку выдаётся предупреждение V575. Кстати, если на одну и ту же строчку программы выдаётся два или более диагностических сообщения, это повод очень-очень внимательно её проверить. Очень часто, одна ошибка делает код подозрительным «с разных точек зрения».

Если внимательно присмотреться к коду, то можно увидеть, что одна закрывающаяся скобка стоит не на своём месте. В результате третьим фактическим аргументом функции является выражение «sizeof(zero_clientaddr) == 0».

Идентичные ошибки можно видеть в соседних функциях. Видимо ошибка размножилась благодаря копированию кода.

Другие проблемные места:

  • pgstatfuncs.c 976
  • pgstatfuncs.c 1023

2. Число в восьмеричной системе счисления

void RestoreArchive(Archive *AHX)
{
  ....
  AHX->minRemoteVersion = 070100;
  AHX->maxRemoteVersion = 999999;
  ....
}

Предупреждение выданное PVS-Studio: V536 Be advised that the utilized constant value is represented by an octal form. Oct: 070100, Dec: 28736. pg_dump pg_backup_archiver.c 301

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

fout->minRemoteVersion = 70000;

Скорее всего, ноль был дописан для красоты. Но из-за этого нуля, число «070100» записано в восьмеричной системе счисления и равно 28736.

3. Классика. Ошибки работы с типом SOCKET

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

typedef UINT_PTR SOCKET;
typedef SOCKET pgsocket;

static int
ident_inet(hbaPort *port)
{
  ....
  pgsocket  sock_fd;
  ....
  sock_fd = socket(ident_serv->ai_family,
                   ident_serv->ai_socktype,
                   ident_serv->ai_protocol);
  if (sock_fd < 0)
  {
  ....
}

Предупреждение выданное PVS-Studio: V547 Expression 'sock_fd < 0' is always false. Unsigned type value is never < 0. postgres auth.c 1668

Проверка (sock_fd < 0) не имеет смысла. Беззнаковая переменная не может быть меньше нуля. Код, обрабатывающий ситуацию, когда не удаётся открыть сокет, никогда не получит управление.

Есть ещё несколько ошибок, идентичных этой:

  • auth.c 1748
  • auth.c 2567
  • pqcomm.c 395
  • pqcomm.c 633
  • postmaster.c 2168

4. Ошибка стирания приватных данных

Найдено много типовых ошибок, связанных с затиранием памяти, содержащих приватные данные. Эта ошибка, пожалуй, встречается ещё чаще, чем проблемы с типом SOCKET.

char *
px_crypt_md5(const char *pw, const char *salt,
             char *passwd, unsigned dstlen)
{
  ....
  unsigned char final[MD5_SIZE];
  ....
  /* Don't leave anything around in vm they could use. */
  memset(final, 0, sizeof final);
  ....
}

Предупреждение выданное PVS-Studio: 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. pgcrypto crypt-md5.c 157

Комментарий подчёркивает, что нужно затереть определённую область памяти. Однако этого не произойдет. Компилятор выбросит вызов функции memset(). Почему это произойдёт и как с этим бороться, можно прочитать в описании диагностического правила.

Мест, где данные не будут стёрты, достаточно много:

  • fortuna.c 294
  • fortuna.c 344
  • fortuna.c 381
  • internal.c 671
  • pgp-encrypt.c 131
  • pgp-encrypt.c 493
  • pgp-encrypt.c 555
  • pgp-decrypt.c 283
  • pgp-decrypt.c 398
  • pgp-decrypt.c 399
  • pgp-decrypt.c 496
  • pgp-decrypt.c 706
  • pgp-decrypt.c 795
  • pgp-pgsql.c 90
  • pgp-pgsql.c 132
  • px-crypt.c 161
  • pgp-pubkey.c 153
  • pgp-pubkey.c 294
  • pgp-pubkey.c 295

Каждое такое место — потенциальная уязвимость! Не затёртые данные самым неожиданным образом могут оказаться записаны на диск или отправлены по сети. Статья на эту тему: Перезаписывать память — зачем?

5. Неопределённое поведение

static char *
inet_cidr_ntop_ipv6(const u_char *src, int bits,
                    char *dst, size_t size)
{
  ....
  m = ~0 << (8 - b);
  ....
}

Предупреждение выданное PVS-Studio: V610 Undefined behavior. Check the shift operator '<<. The left operand '~0' is negative. postgres inet_cidr_ntop.c 206

Нельзя сдвигать отрицательные числа. Это приводит к неопределённому поведению. Подробнее: "Не зная брода, не лезь в воду — часть третья".

Другие опасные сдвиги:

  • network.c 1435
  • signal.c 118
  • signal.c 125
  • varbit.c 1508
  • varbit.c 1588

6. Опечатка

static void
AddNewRelationTuple(....)
{
  ....
  new_rel_reltup->relfrozenxid = InvalidTransactionId;
  new_rel_reltup->relfrozenxid = InvalidMultiXactId;
  ....
}

Предупреждение выданное PVS-Studio: V519 The 'new_rel_reltup->relfrozenxid' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 912, 913. postgres heap.c 913

Одной переменной присваивается два разных значения. Кажется это опечатка. Возможно, во второй строчке, значение должно быть присвоено переменной 'new_rel_reltup->relminmxid'

Заключение

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

И желаю радостного Нового Года!

Автор: Andrey2008

Источник

* - обязательные к заполнению поля


https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js