Forwarding this to pgsql-bugs, since this isn't a security issue, as
pg_ctl can only be called an admin. My replies inline.
-------- Original Message --------
Subject: [pgsql-security] race in pg_ctl start -w
Date: Thu, 11 Oct 2012 12:39:02 -0400
From: Dave Vitek <dvi...@grammatech.com>
To: <secur...@postgresql.org>
Hi,
I don't really think this is a security issue since pg_ctl isn't really
an appealing target, but in theory you could hijack the process if you
got very very lucky. Feel free to forward this to the bugs list if you
decide it isn't sensitive.
We recently ran into a crash when using pg_ctl to start the postgres
daemon. The command was:
pg_ctl start -w -t 600 -D
It was a segv inside the body of malloc and didn't want to repro.
The good news is, we develop a static analysis tool called CodeSonar for
detecting bugs like this. It has been flagging the problem since 2009,
but we've generally been ignoring reports in third-party code (there are
a lot of them). We analyze postgres on a regular basis because it is
used by CS's web UI.
So, the problem is that there's a race condition in the readfile()
function that pg_ctl uses to read postmaster.pid. It does essentially this:
1. open postmaster.pid
2. Read through the file one byte at a time, and count newlines.
3. Allocate buffers to hold that many lines.
4. Rewind and read the file again, this time copying the lines to the
allocated buffers
The race condition is that if another process (postmaster) changes the
file between steps 2 and 4, so that there are now more lines, reading
the file again can overrun the buffers allocated. In particular that can
happen at postmaster startup, when postmaster initially creates the file
empty, and then does a write() to fill it in.
It seems very unlucky if you actually hit that race condition, but I
guess it's not impossible.
A straightforward fix would be to just allocate one large-enough buffer
to begin with, e.g 8k, and read the whole file in one go. I'll write up
a patch for that.
It also flagged a very similar issue (looks like the code was copied &
pasted) in initdb.c. I haven't looked into whether that one is as
likely to be subject to a race as the pg_ctl one, but it could be
problematic as well.
I don't think it's a problem for initdb, because the files that it reads
should not change on the fly. Nevertheless, I guess we might as well add
a check there.
Here are the bug reports:
http://www.grammatech.com/products/codesonar/examples/pg_ctl_race.html
http://www.grammatech.com/products/codesonar/examples/initdb_race.html
I've just saved static HTML above, so none of the links, navigation
features, or images will work. The files are posted on the website
because they seemed a bit big for sending to a list. I haven't shared
them with anyone else outside the company.
- Dave
-. Heikki
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
