Probably security hole in postgresql-7.4.1

From: Ken Ashcraft <ken(at)coverity(dot)com>
To: security(at)postgresql(dot)org
Subject: Probably security hole in postgresql-7.4.1
Date: 2004-04-19 22:14:09
Message-ID: 1082412849.6563.18.camel@dns.coverity.int
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I work at Coverity where we use static analysis to find bugs in
software. I ran a security checker over postgresql-7.4.1 and I think I
found a security hole. I'm not familiar with the postgres source, so
this report may be false. My interpretation of the code follows.

I'd appreciate your feedback,
Ken Ashcraft

In the code below, fld_size gets copied in from a user specified file.
It is passed as the 'needed' parameter to enlargeStringInfo(). If
needed is a very large positive value, the addition 'needed += str->len
+ 1;' could cause an overflow, making needed a negative number.
enlargeStringInfo() would quickly return, thinking that enough memory
was present. However, the call to CopyGetData() uses the large,
positive fld_size, resulting in a buffer overflow, assuming that the
user's file has enough data.

/home/kash/user-progs/postgres/postgresql-7.4.1/src/backend/commands/copy.c:2050:CopyReadBinaryAttribute: ERROR:TAINT: 2030:2050:Passing unbounded user value "fld_size" as arg 1 to function "enlargeStringInfo", which uses it unsafely in model [SOURCE_MODEL=(lib,CopyGetInt32,user,taint)] [SINK_MODEL=(lib,enlargeStringInfo,user,trustingsink)] [BOUNDS= Upper bound on line 2040] [PATH=]

bool *isnull)
{
int32 fld_size;
Datum result;

Start --->
fld_size = CopyGetInt32();

... DELETED 14 lines ...

/* reset attribute_buf to empty, and load raw data in it */
attribute_buf.len = 0;
attribute_buf.data[0] = '\0';
attribute_buf.cursor = 0;

Error --->
enlargeStringInfo(&attribute_buf, fld_size);

CopyGetData(attribute_buf.data, fld_size);
if (CopyGetEof())
-------------------

enlargeStringInfo(StringInfo str, int needed)
{
int newlen;

needed += str->len + 1; /* total space required now */
if (needed <= str->maxlen)
return; /* got enough space already */

/*
* We don't want to allocate just a little more space with each
* append; for efficiency, double the buffer size each time it
* overflows. Actually, we might need to more than double it if
* 'needed' is big...
*/
newlen = 2 * str->maxlen;
while (needed > newlen)
newlen = 2 * newlen;

str->data = (char *) repalloc(str->data, newlen);

str->maxlen = newlen;
}

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message L J Bayuk 2004-04-19 23:50:48 Re: [HACKERS] Why is libpgtcl still in CVS?
Previous Message Bruce Momjian 2004-04-19 21:22:59 Re: 'begin transaction' new syntax bug?