Oops in fe-auth.c - Mailing list pgsql-hackers

From Magnus Hagander
Subject Oops in fe-auth.c
Date
Msg-id 20070723133621.GH29554@svr2.hagander.net
Whole thread Raw
Responses Re: Oops in fe-auth.c  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: "Florian G. Pflug"
Date:
Subject: Re: Full page images in WAL & Cache Invalidation
Next
From: Tom Lane
Date:
Subject: Re: Oops in fe-auth.c