Thread: Probably security hole in postgresql-7.4.1

Probably security hole in postgresql-7.4.1

From
Ken Ashcraft
Date:
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:Passingunbounded 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
boundon 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
timeit * 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;
}




Re: Probably security hole in postgresql-7.4.1

From
Tom Lane
Date:
Ken Ashcraft <ken@coverity.com> writes:
> 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.
>
> 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. 

I've applied a patch that fixes this issue, as well as the related one
that enlargeStringInfo could go into an infinite loop.

Although the path of control you identify doesn't seem very threatening
(since one must already be superuser to execute COPY from a file), the
same sort of problem could be triggered by sending a malformed data
packet, thus opening up the problem to anyone who can get past the
initial postmaster authentication check.  So this is more severe than we
first thought.

If you are looking to improve your checker, you might want to look into
why it only found this path for bad data, and not the path leading from
the client connection socket.  Seems like it should've found that too.

Thanks for the report!
        regards, tom lane


Re: Probably security hole in postgresql-7.4.1

From
Bruce Momjian
Date:
Tom Lane wrote:
> Ken Ashcraft <ken@coverity.com> writes:
> > 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.
> >
> > 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. 
> 
> I've applied a patch that fixes this issue, as well as the related one
> that enlargeStringInfo could go into an infinite loop.
> 
> Although the path of control you identify doesn't seem very threatening
> (since one must already be superuser to execute COPY from a file), the
> same sort of problem could be triggered by sending a malformed data
> packet, thus opening up the problem to anyone who can get past the
> initial postmaster authentication check.  So this is more severe than we
> first thought.
> 
> If you are looking to improve your checker, you might want to look into
> why it only found this path for bad data, and not the path leading from
> the client connection socket.  Seems like it should've found that too.

Should we be thinking about a 7.4.3?  I think we are waiting for
pg_autovacuum fixes though.

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 


Re: Probably security hole in postgresql-7.4.1

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Should we be thinking about a 7.4.3?

I'm not panicking over this particular bug ... but it does seem like
we've accumulated enough fixes since 7.4.2 that it may be time to start
thinking about another dot-release.  Maybe set a date towards the end of
the month?
        regards, tom lane


Re: Probably security hole in postgresql-7.4.1

From
Shachar Shemesh
Date:
Tom Lane wrote:

>Bruce Momjian <pgman@candle.pha.pa.us> writes:
>  
>
>>Should we be thinking about a 7.4.3?
>>    
>>
>
>I'm not panicking over this particular bug ... but it does seem like
>we've accumulated enough fixes since 7.4.2 that it may be time to start
>thinking about another dot-release.  Maybe set a date towards the end of
>the month?
>
>            regards, tom lane
>  
>
Industry practices dictate that we do issue SOMETHING now. The bug is 
now public, and can be exploited.

This does not necessarily have to be 7.4.3. We can issue 7.4.2.1, 
containing only this fix, so that people who need to expose their 
database are not left open to attacks.

Also, if we want greater flexibility in handling these cases in the 
future, we should set up an invite-only list for reporting security 
bugs, and advertise it on the web site as the place to report security 
issues. Had this vulnerability been reported there, we could reasonably 
hold on without releasing a fix until 7.4.3 was ready.

If you need help in that list, I have a lot of experience with code 
security, but very little experience with the Postgresql code. Also, it 
would be a good idea to invite all the distro-packagers to be on that list.
            Shachar

-- 
Shachar Shemesh
Lingnu Open Source Consulting
http://www.lingnu.com/



Re: Probably security hole in postgresql-7.4.1

From
Greg Stark
Date:
Shachar Shemesh <psql@shemesh.biz> writes:

> Also, if we want greater flexibility in handling these cases in the future, we
> should set up an invite-only list for reporting security bugs, and advertise it
> on the web site as the place to report security issues. Had this vulnerability
> been reported there, we could reasonably hold on without releasing a fix until
> 7.4.3 was ready.

A lot of people would be unhappy with that approach. A) they don't know the
people on the invite-only list and have no basis to trust them and B) Often
when a white hat reports the problem the black hats have known about it for
much longer already.

-- 
greg



Re: Probably security hole in postgresql-7.4.1

From
Bruno Wolff III
Date:
On Wed, May 12, 2004 at 10:46:00 +0300, Shachar Shemesh <psql@shemesh.biz> wrote:
> Industry practices dictate that we do issue SOMETHING now. The bug is 
> now public, and can be exploited.

The description of the problem indicates that it can only be exploited
after you have authenticated to the database. Since people who can
connect to a postgres database can already cause denial of service
attacks, this problem isn't a huge deal. It makes breaches in other
programs (web server process especially) worse and provides another
way for authorized users to cause problems.

A release should probably be made soon, as a way to advertise the problem
so that people are aware of it and can take appropiate steps. I don't think
that this problem warrants bypassing normal minor release proceedure.


Re: Probably security hole in postgresql-7.4.1

From
Tom Lane
Date:
Greg Stark <gsstark@mit.edu> writes:
> Shachar Shemesh <psql@shemesh.biz> writes:
>> Also, if we want greater flexibility in handling these cases in the future, we
>> should set up an invite-only list for reporting security bugs,

> A lot of people would be unhappy with that approach. A) they don't know the
> people on the invite-only list and have no basis to trust them and B) Often
> when a white hat reports the problem the black hats have known about it for
> much longer already.

Past procedure for sensitive bugs has been for people to send reports to
the core committee (pgsql-core at postgresql dot org).  If you don't
trust us you probably shouldn't be using Postgres ;-)

As per other comments, I don't find this bug compelling enough to
justify an instant release.  Also, the original reporter is still
running his analysis tool and has found some other things that might
be worth patching.  (Again, nothing compelling yet... but ...)
I'm inclined to wait a bit longer and see if we can't include some
more fixes in 7.4.3.
        regards, tom lane


Re: Probably security hole in postgresql-7.4.1

From
Shachar Shemesh
Date:
Bruno Wolff III wrote:

>On Wed, May 12, 2004 at 10:46:00 +0300,
>  Shachar Shemesh <psql@shemesh.biz> wrote:
>  
>
>>Industry practices dictate that we do issue SOMETHING now. The bug is 
>>now public, and can be exploited.
>>    
>>
>
>The description of the problem indicates that it can only be exploited
>after you have authenticated to the database. Since people who can
>connect to a postgres database can already cause denial of service
>attacks, this problem isn't a huge deal.
>
My take on this is different. To me, a DoS is a nuisance, but an 
arbitrary code execution vulnerability means information leak, and a 
major escalation (from which further escalation may be possible).

> It makes breaches in other
>programs (web server process especially) worse and provides another
>way for authorized users to cause problems.
>  
>
Not to mention being another chain.

>A release should probably be made soon, as a way to advertise the problem
>so that people are aware of it and can take appropiate steps. I don't think
>that this problem warrants bypassing normal minor release proceedure.
>  
>
Ok. How about an official patch against 7.4.2 that fixes it, so that 
packagers can make their own informed decision. Also, has anybody 
checked what other versions are affected? Is 7.3? 7.2? Some people can't 
afford to upgrade due to data inconsistancy.

For those reasons I suggested a seperate mailing list.
         Shachar

-- 
Shachar Shemesh
Lingnu Open Source Consulting
http://www.lingnu.com/



Re: Probably security hole in postgresql-7.4.1

From
Tom Lane
Date:
Shachar Shemesh <psql@shemesh.biz> writes:
> Ok. How about an official patch against 7.4.2 that fixes it, so that 
> packagers can make their own informed decision.

The "official patch" is available to anyone who wants it from our CVS
server.
http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/backend/lib/stringinfo.c.diff?r1=1.36&r2=1.36.4.1

BTW, all the principal packagers read this list and have doubtless made
their informed decisions already ...

> Also, has anybody checked what other versions are affected?

Nothing before 7.4, at least by the known implications of this issue.
Again, if we wait a while and let Ken keep running his analysis tool,
he might turn up other stuff we need to fix.  Maybe even stuff that
needs a fix much worse than this does.

>>>Industry practices dictate that we do issue SOMETHING now. The bug is 
>>>now public, and can be exploited.

I frankly think that this discussion is emblematic of all the worst
tendencies of the security community.  Have you forgotten the fable
about the boy who cried "wolf"?  If you demand a Chinese fire drill
for every issue that could conceivably be exploited, you'll soon find
yourself unable to get peoples' attention for problems that are really
serious.

I repeat: in my estimation this is not a bug that needs a fix yesterday.
AFAICS it would be very difficult to cause more than a nuisance DOS with
it, and there are plenty of other ways for authenticated database users
to cause those.
        regards, tom lane


Re: Probably security hole in postgresql-7.4.1

From
Bruno Wolff III
Date:
On Wed, May 12, 2004 at 23:36:49 +0300, Shachar Shemesh <psql@shemesh.biz> wrote:
> 
> My take on this is different. To me, a DoS is a nuisance, but an 
> arbitrary code execution vulnerability means information leak, and a 
> major escalation (from which further escalation may be possible).

A DOS is generally more than a nuisance in production environments.
In most cases you aren't going to be giving direct access to your DB
to people that aren't fairly trusted. The exception may be some of
the web hosting places that provide DB backed web pages.

> Not to mention being another chain.

This isn't very significant as you have to authenticate to the DB first
to exploit it. That's a lot less of a problem than something directly
accessible by anyone from the net such as a web server.

> Ok. How about an official patch against 7.4.2 that fixes it, so that 
> packagers can make their own informed decision. Also, has anybody 
> checked what other versions are affected? Is 7.3? 7.2? Some people can't 
> afford to upgrade due to data inconsistancy.

Hasn't it already been committed to 7.4 stable? If so, just grab an update
from CVS.

Something should probably be done about 7.2 and 7.3 if the same bug exists
in those versions. Nobody should be running anything earlier than that.


Re: Probably security hole in postgresql-7.4.1

From
Shachar Shemesh
Date:
Tom Lane wrote:

>Shachar Shemesh <psql@shemesh.biz> writes:
>  
>
>>Also, has anybody checked what other versions are affected?
>>    
>>
>
>Nothing before 7.4, at least by the known implications of this issue.
>Again, if we wait a while and let Ken keep running his analysis tool,
>he might turn up other stuff we need to fix.  Maybe even stuff that
>needs a fix much worse than this does.
>
>  
>
and also

>I frankly think that this discussion is emblematic of all the worst
>tendencies of the security community.  Have you forgotten the fable
>about the boy who cried "wolf"?
>  
>
I totally agree. That's why I suggested preventing the automatic public 
disclosure for Ken's next bugs, as well as anyone else's. This way, if 
we do need a few extra days, we can have them while still limiting the 
window of exposure.

>I repeat: in my estimation this is not a bug that needs a fix yesterday.
>AFAICS it would be very difficult to cause more than a nuisance DOS with
>it, and there are plenty of other ways for authenticated database users
>to cause those.
>  
>
I'm sorry. Maybe it's spending too many years in the security industry 
(I've been Check Point's "oh my god we have a security problem" process 
manager for over two years). Maybe it's knowing how to actually exploit 
these problems. Maybe it's just seeing many of the good guys (OpenBSD's 
Theo included) fall flat on their faces after saying "This is a DoS 
only". In my book, a buffer overrun=arbitrary code execution.

For a now famous example of a bug declared "non exploitable", followed 
by an exploit, see http://www.theinquirer.net/?article=4053. I have been 
on the mailing lists at the time. The problem was declared 
"unexploitable on i386" by some of the best known names in the security 
industry of the time.

>            regards, tom lane
>  
>
Please. I'm not saying "Release now". I'm saying "get a mechanism for 
smarter handling of future events".
         Shachar

-- 
Shachar Shemesh
Lingnu Open Source Consulting
http://www.lingnu.com/



Re: Probably security hole in postgresql-7.4.1

From
Bruno Wolff III
Date:
On Thu, May 13, 2004 at 00:54:19 +0300, Shachar Shemesh <psql@shemesh.biz> wrote:
> >
> I'm sorry. Maybe it's spending too many years in the security industry 
> (I've been Check Point's "oh my god we have a security problem" process 
> manager for over two years). Maybe it's knowing how to actually exploit 
> these problems. Maybe it's just seeing many of the good guys (OpenBSD's 
> Theo included) fall flat on their faces after saying "This is a DoS 
> only". In my book, a buffer overrun=arbitrary code execution.

But it is still a local user exploit, not a remote user exploit. That makes
a big difference in how the bug should be treated.


Re: Probably security hole in postgresql-7.4.1

From
"Ken Ashcraft"
Date:
> Ken Ashcraft <ken@coverity.com> writes:
>> 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.
>>
>> 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.
>
> I've applied a patch that fixes this issue, as well as the related one
> that enlargeStringInfo could go into an infinite loop.
>
> Although the path of control you identify doesn't seem very threatening
> (since one must already be superuser to execute COPY from a file), the
> same sort of problem could be triggered by sending a malformed data
> packet, thus opening up the problem to anyone who can get past the
> initial postmaster authentication check.  So this is more severe than we
> first thought.
>

Great.  Thanks for the feedback.  If it is serious, is an advisory in order?

Ken


Re: Probably security hole in postgresql-7.4.1

From
Tom Lane
Date:
"Ken Ashcraft" <ken@coverity.com> writes:
>> ... thus opening up the problem to anyone who can get past the
>> initial postmaster authentication check.  So this is more severe than we
>> first thought.

> Great.  Thanks for the feedback.  If it is serious, is an advisory in order?

No, we'll just push out the fix as part of the next update version
(though that may happen a little sooner than it would have otherwise).
Sensible people don't give direct database connections to untrustworthy
users in the first place, since there are so many ways you can cause
problems if you can issue random SQL commands ...
        regards, tom lane