Год заканчивается, а я давно не писал заметок о проверке открытых проектов. Мне уже неоднократно предлагали проверить проект PostgreSQL Database Management System. Этим я и занялся. К сожалению, грандиозной и интересной статьи не получится. Я заметил только несколько типовых ошибок. Так что в этот раз получилась совсем небольшая статья.
----
PostgreSQL - свободная объектно-реляционная система управления базами данных (СУБД). PostgreSQL базируется на языке SQL и поддерживает многие из возможностей стандарта SQL:2003 (ISO/IEC 9075). Подробнее о проекте можно почитать на сайте Wikipedia и на самом сайте проекта.
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".
Идентичные ошибки можно видеть в соседних функциях. Видимо ошибка размножилась благодаря копированию кода.
Другие проблемные места:
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.
В операционной системе Windows тип 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) не имеет смысла. Беззнаковая переменная не может быть меньше нуля. Код, обрабатывающий ситуацию, когда не удаётся открыть сокет, никогда не получит управление.
Есть ещё несколько ошибок, идентичных этой:
Найдено много типовых ошибок, связанных с затиранием памяти, содержащих приватные данные. Эта ошибка, пожалуй, встречается ещё чаще, чем проблемы с типом 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(). Почему это произойдёт и как с этим бороться, можно прочитать в описании диагностического правила.
Мест, где данные не будут стёрты, достаточно много:
Каждое такое место - потенциальная уязвимость! Не затёртые данные самым неожиданным образом могут оказаться записаны на диск или отправлены по сети. Статья на эту тему: Перезаписывать память - зачем?
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
Нельзя сдвигать отрицательные числа. Это приводит к неопределённому поведению. Подробнее: "Не зная брода, не лезь в воду - часть третья".
Другие опасные сдвиги:
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, мы готовы предоставить им на время регистрационный ключ. Прошу написать их нам в поддержку.
И желаю радостного Нового Года!