Re: libpq minor TOCTOU violation
Aleksander Alekseev <aleksander@timescale.com>
From: Aleksander Alekseev <aleksander@timescale.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Peter Eisentraut <peter@eisentraut.org>
Date: 2024-08-12T12:34:52Z
Lists: pgsql-hackers
Hi, > I was playing with a static analyzer security scanner and it flagged a > time-of-check-time-of-use violation in libpq. I was going to propose a > fix for this on -hackers, since you probably can't do anything > interesting with this, but then I figured I'd better check here first. > > libpq checks the permissions of the password file before opening it and > rejects it if the permissions are more permissive than 0600. It does > this in two separate steps, first stat(), then fopen(), which is what > triggers the static analyzer. The standard fix for this kind of thing > is to open the file first and then use fstat() on the file handle for > the permission check. > > Note that libpq doesn't check who the owner of the file is or what > directory it is in. The location of the password file can be changed > from its default via libpq connection settings. So it seems to me that > if the location of the password file were a world-writable directory, an > "attacker" could supply a dummy file with 0600 permissions for the > stat() call and then swap it out for a world-readable file with > passwords for the fopen() call, both files owned by the attacker. And > so they could have the libpq application try out passwords on their > behalf. Obviously, this requires a number of unlikely circumstances, > including the ability to repeatedly exploit the gap between the stat() > and fopen() call. But it seems formally incorrect, so it seems good to > fix it, at least to make the code a better example. Not entirely sure about the presence of a serious security issue but silencing a static analyzer sounds like a good idea, especially since the fix is simple. -- Best regards, Aleksander Alekseev
Commits
-
libpq: Fix minor TOCTOU violation
- e882bcae032d 18.0 landed