вторник, 24 декабря 2013 г.

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

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

Проверка (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 '

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


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



  • 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, мы готовы предоставить им на время регистрационный ключ. Прошу написать их нам в поддержку.

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


This entry passed through the Full-Text RSS service — if this is your content and you're reading it on someone else's site, please read the FAQ at fivefilters.org/content-only/faq.php#publishers.


Комментариев нет:

Отправить комментарий