Год заканчивается, а я давно не писал заметок о проверке открытых проектов. Мне уже неоднократно предлагали проверить проект 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
