Oops in fe-auth.c

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Oops in fe-auth.c
Date: 2007-07-23 13:36:21
Message-ID: 20070723133621.GH29554@svr2.hagander.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

I've been debugging some really weird crashes in libpq on win32, and I
think I've finally found the reason for the heap corruption that shows up
in msvc debug mode.

When run in debug mode, the runtime for msvc will *zero-pad the entire
buffer* in a strncpy() call. This in itself is not bad (just slow), but it
shows a rather bad bug in libpq.

In a bunch of places in fe-auth.c, we do:
strncpy(PQerrormsg, libpq_gettext("out of memory\n"), PQERRORMSG_LENGTH);

Except when calling it, the size of the buffer is 256 bytes. But
PQERRORMSG_LENGTH is 1024.

Naturally, this causes a heap corruption. It doesn't happen in production,
because the string length fits as long as there is no padding.

One way to get around this on win32 is to just use snprintf() instead of
strncpy(), since it doesn't pad. But that's just hiding the underlying
problem, so I think that's a really bad fix.

I assume the comment in the header:
* NOTE: the error message strings returned by this module must not
* exceed INITIAL_EXPBUFFER_SIZE (currently 256 bytes).

refers to this, but it's hard to guarantee that from the code since it's
translated strings.

I see a comment in fe-connect.c that has
* XXX fe-auth.c has not been fixed to support PQExpBuffers,

Given this, I'll go ahead and fix fe-connect to support PQExpBuffers,
unless there are any objections. Also, is this something we shuold
backpatch - or just ignore since we've had no actual reports of it in the
field?

//Magnus

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2007-07-23 14:12:13 Re: [HACKERS] Oops in fe-auth.c
Previous Message Florian G. Pflug 2007-07-23 13:32:00 Re: Full page images in WAL & Cache Invalidation

Browse pgsql-patches by date

  From Date Subject
Next Message Magnus Hagander 2007-07-23 14:12:13 Re: [HACKERS] Oops in fe-auth.c
Previous Message Dave Page 2007-07-23 13:29:03 Re: COPYable logs