Out-of-bounds write and incorrect detection of trigger file in pg_standby

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Out-of-bounds write and incorrect detection of trigger file in pg_standby
Date: 2015-01-14 13:24:04
Message-ID: CAB7nPqStzuOrrguB381+WbNkparC7j3PDZEacAqbCsbDtrGqRg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

In pg_standby.c, we use a 32-byte buffer in CheckForExternalTrigger to
which is read the content of the trigger file defined by -f:
CheckForExternalTrigger(void)
{
char buf[32];
[...]
if ((len = read(fd, buf, sizeof(buf))) < 0)
{
stuff();
}

if (len < sizeof(buf))
buf[len] = '\0';
However it happens that if the file read contains 32 bytes or more, we
enforce \0 in a position out of bounds. While looking at that, I also
noticed that pg_standby can trigger a failover as long as the
beginning of buf matches either "smart" or "fast", but I think we
should check for a perfect match instead of a comparison of the first
bytes, no?

Attached is a patch to fix the out-of-bound issue as well as
improvements for the detection of the trigger file content. I think
that the out-of-bound fix should be backpatched, while the
improvements for the trigger file are master-only. Coverity has
pointed out the out-of-bound issue.
Regards,
--
Michael

Attachment Content-Type Size
20150114_pg_standby_fixes.patch text/x-patch 697 bytes

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2015-01-14 13:24:59 Re: Turning recovery.conf into GUCs
Previous Message Andres Freund 2015-01-14 11:19:36 Re: Minor configure tweak to simplify adjusting gcc warnings