PostgreSQL New Year Check

The year ends, and I haven’t written notes about checking open projects for a long time. I have been repeatedly offered to check the PostgreSQL Database Management System project. This is what I do. Unfortunately, an ambitious and interesting article will not work. I noticed only a few sample errors. So this time it turned out quite a small article.



PostgreSQL is a free object-relational database management system (DBMS). PostgreSQL is based on SQL and supports many of the features of the SQL: 2003 standard (ISO / IEC 9075). More information about the project can be read on the Wikipedia website and on the project website itself.
')

1. Typos when using the memcmp () function

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

PVS-Studio warning : V526 The 'memcmp' function returns 0 if the corresponding buffers are equal. Consider examining the condition for mistakes. postgres pgstatfuncs.c 712

Also on this line is a warning V575 . By the way, if two or more diagnostic messages are issued for the same program line, this is a reason to check it very, very carefully. Very often, one mistake makes the code suspicious "from different points of view."

If you look closely at the code, you can see that one closing bracket is not in its place. As a result, the third actual function argument is the expression sizeof (zero_clientaddr) == 0.

Identical errors can be seen in adjacent functions. Apparently the error multiplied by copying the code.

Other problem areas:

• pgstatfuncs.c 976
• pgstatfuncs.c 1023

2. Number in octal number system

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

The warning issued by PVS-Studio: V536 Be advised that the utilized form is. Oct: 070100, Dec: 28736. pg_dump pg_backup_archiver.c 301

If you look at the code located nearby, then we can conclude that the programmer did not plan to use the number written in the octal number system at all. For example, there is this line:

 fout->minRemoteVersion = 70000; 

Most likely, zero was added for beauty. But because of this zero, the number “070100” is written in the octal number system and is equal to 28736.

3. Classic. Errors of work with type SOCKET

In the Windows operating system, the SOCKET type is the unsigned type. About this, many do not know or forget. As a result, the same mistake can be seen in different projects. This error was not overtaken by the PostgreSQL project.

 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) { .... } 

Warning given by PVS-Studio: V547 Expression 'sock_fd <0' is always false. Unsigned type value is never <0. postgres auth.c 1668

Check (sock_fd <0) does not make sense. An unsigned variable cannot be less than zero. The code that handles the situation when it fails to open a socket will never get control.

There are some more mistakes identical to this:

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

4. Error erasing private data

Found a lot of typical errors associated with mashing the memory containing private data. This error is perhaps more common than problems with the SOCKET type.

 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); .... } 

The warning issued by PVS-Studio: V597 The compiler could delete the memset function call, which is used to flush the final buffer. The RtlSecureZeroMemory () function should be used to erase the private data. pgcrypto crypt-md5.c 157

The comment emphasizes that you need to wipe a certain area of ​​memory. However, this will not happen. The compiler will throw a call to the memset () function. Why this happens and how to deal with it can be found in the description of the diagnostic rule.

There are a lot of places where the data will not be erased:

• 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

Every such place is a potential vulnerability! Data that has not been erased may unexpectedly be written to disk or sent over the network. An article on this topic: Overwrite memory - why?

5. Undefined behavior

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

Alert issued by PVS-Studio: V610 Undefined behavior. Check the shift operator '<<. The left operand '~ 0' is negative. postgres inet_cidr_ntop.c 206

You can not move negative numbers. This leads to undefined behavior. More: " Without knowing the ford, do not climb into the water - part three ."

Other dangerous shifts:

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

6. Typo

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

The warning issued by 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

One variable is assigned two different values. It seems to be a typo. Perhaps in the second line, the value should be assigned to the variable 'new_rel_reltup-> relminmxid'

Conclusion

If any of the PostgreSQL project developers are interested in the PVS-Studio analyzer, we are ready to provide them with a registration key for the time being. Please write them to us in support.

And I wish you a happy New Year!