@(#)Mordred Labs advisory 0x0003: Buffer overflow in PostgreSQL (fwd)

Vince Vielhaber <vev@michvhf.com> writes:

Here's yet another. He claims malicious code can be run on the server
with this one.

regression=# select repeat('xxx',1431655765);
server closed the connection unexpectedly

This is probably caused by integer overflow in calculating the size of
the repeat's result buffer. It'd take some considerable doing to create
an arbitrary-code exploit, but perhaps could be done. Anyone want to
investigate a patch? I think we likely need something like

	bufsize = input_length * repeats;
+	/* check for overflow in multiplication */
+	if (bufsize / repeats != input_length)
+		elog(ERROR, "repeat result too long");

but I haven't thought it through in detail.

regards, tom lane

On Tue, 20 Aug 2002, Tom Lane wrote:

Vince Vielhaber <vev@michvhf.com> writes:

Here's yet another. He claims malicious code can be run on the server
with this one.

regression=# select repeat('xxx',1431655765);
server closed the connection unexpectedly

This is probably caused by integer overflow in calculating the size of
the repeat's result buffer. It'd take some considerable doing to create
an arbitrary-code exploit, but perhaps could be done. Anyone want to
investigate a patch? I think we likely need something like

bufsize = input_length * repeats;
+	/* check for overflow in multiplication */
+	if (bufsize / repeats != input_length)
+		elog(ERROR, "repeat result too long");

but I haven't thought it through in detail.

Where do we check that this:

result = (text *) palloc(tlen);

is even successful? Is it in one of the macros (VARATT_SIZEP or VARDATA)?
It appears like it goes into the count and copy regardless.

Vince.
--
==========================================================================
Vince Vielhaber -- KA8CSH email: vev@michvhf.com http://www.pop4.net
56K Nationwide Dialup from $16.00/mo at Pop4 Networking
http://www.camping-usa.com http://www.cloudninegifts.com
http://www.meanstreamradio.com http://www.unknown-artists.com
==========================================================================

Vince Vielhaber <vev@michvhf.com> writes:

Where do we check that this:
result = (text *) palloc(tlen);
is even successful?

palloc elogs if it can't allocate the space; it's unlike malloc in that
respect. I believe it also has a guard to reject requests > 1Gb, so
I think it's reasonably proof against internal arithmetic overflows.

This problem is strictly repeat's error, not palloc's.

regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> writes:

Vince Vielhaber <vev@michvhf.com> writes:

Here's yet another. He claims malicious code can be run on the server
with this one.

regression=# select repeat('xxx',1431655765);
server closed the connection unexpectedly

This is probably caused by integer overflow in calculating the size of
the repeat's result buffer. It'd take some considerable doing to create
an arbitrary-code exploit, but perhaps could be done. Anyone want to
investigate a patch?

This seems to fix the problem:

nconway=# select repeat('xxx',1431655765);
ERROR: Requested buffer is too large.

It uses the logic you suggested. Just a quick and dirty fix, I may
have missed something... The patch applies cleanly to both CVS HEAD
and the 7.2 stable branch.

Cheers,

Neil

--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

Neil Conway <neilc@samurai.com> writes:

+ 	/* Check for integer overflow */
+ 	if (tlen / slen != count)
+ 		elog(ERROR, "Requested buffer is too large.");

What about slen == 0?

regards, tom lane

Vince Vielhaber <vev@michvhf.com> writes:

Here's yet another.

Should someone from the core team perhaps get in contact with this guy
and ask if he could get in contact with the development team before
publicizing any further security holes? AFAIK that is standard
operating procedure in most cases...

Second, it might be worth pushing a 7.2.2 release containing the fix
for this bug, as well as the datetime problem. If that sounds
reasonable to the people who have to do the most work on a new release
(e.g. Marc), I can volunteer to backport a fix for the datetime
problem.

Cheers,

Neil

--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

On 20 Aug 2002, Neil Conway wrote:

Vince Vielhaber <vev@michvhf.com> writes:

Here's yet another.

Should someone from the core team perhaps get in contact with this guy
and ask if he could get in contact with the development team before
publicizing any further security holes? AFAIK that is standard
operating procedure in most cases...

I just asked him/her that on bugtraq.

Vince.
--
==========================================================================
Vince Vielhaber -- KA8CSH email: vev@michvhf.com http://www.pop4.net
56K Nationwide Dialup from $16.00/mo at Pop4 Networking
http://www.camping-usa.com http://www.cloudninegifts.com
http://www.meanstreamradio.com http://www.unknown-artists.com
==========================================================================

Vince Vielhaber wrote:

Here's yet another. He claims malicious code can be run on the server
with this one.

--[ Solution

Do you still running postgresql? ...Can't believe that...
If so, execute the following command as a root: "killall -9 postmaster",
and wait until the patch will be available.

You've got to love this last part. ;-)

This has the attitude of "Oh, look at me, I found a bug. Let me
publicize it and get more attention. Aren't you proud of me?"

-- 
  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, Pennsylvania 19073

Dann Corbit wrote:

-----Original Message-----
From: Neil Conway [mailto:neilc@samurai.com]
Sent: Tuesday, August 20, 2002 1:44 PM
To: Vince Vielhaber
Cc: pgsql-hackers@postgreSQL.org
Subject: Re: [HACKERS] @(#)Mordred Labs advisory 0x0003:
Buffer overflow in PostgreSQL (fwd)

Vince Vielhaber <vev@michvhf.com> writes:

Here's yet another.

Should someone from the core team perhaps get in contact with
this guy and ask if he could get in contact with the
development team before publicizing any further security
holes? AFAIK that is standard operating procedure in most cases...

As long as we continue to find out about them, I would just let him work
away.
He is clearly an excellent tester, and if you had to hire him it would
be very expensive.

As long as he is producing results of such great value, I think it is
wonderful.

Yea, someone please contact him and tell him to keep going.

-- 
  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, Pennsylvania 19073

-----Original Message-----
From: Neil Conway [mailto:neilc@samurai.com]
Sent: Tuesday, August 20, 2002 1:44 PM
To: Vince Vielhaber
Cc: pgsql-hackers@postgreSQL.org
Subject: Re: [HACKERS] @(#)Mordred Labs advisory 0x0003:
Buffer overflow in PostgreSQL (fwd)

Vince Vielhaber <vev@michvhf.com> writes:

Here's yet another.

Should someone from the core team perhaps get in contact with
this guy and ask if he could get in contact with the
development team before publicizing any further security
holes? AFAIK that is standard operating procedure in most cases...

As long as we continue to find out about them, I would just let him work
away.
He is clearly an excellent tester, and if you had to hire him it would
be very expensive.

As long as he is producing results of such great value, I think it is
wonderful.

Second, it might be worth pushing a 7.2.2 release containing
the fix for this bug, as well as the datetime problem. If
that sounds reasonable to the people who have to do the most
work on a new release (e.g. Marc), I can volunteer to
backport a fix for the datetime problem.

Bugs that cause a catastrophic error (e.g. "crash" of the database
engine, causing loss of data) should have the highest priority. Call
them category zero.
Bugs that cause incorrect results should have the next highest priority.
Call them category one.
Bugs that are minor annoyances (e.g. "appearance" such as a misspelled
word in a help file) should be low priority.
Bugs that are only suggestions for improvements should have the lowest
priority.

All known category zero and one bugs should be fixed before each and
every new release. IMO-YMMV.

Tom Lane <tgl@sss.pgh.pa.us> writes:

Neil Conway <neilc@samurai.com> writes:

+ 	/* Check for integer overflow */
+ 	if (tlen / slen != count)
+ 		elog(ERROR, "Requested buffer is too large.");

What about slen == 0?

Good point -- that wouldn't cause incorrect results or a security
problem, but it would reject input that we should really accept.

Revised patch is attached.

Cheers,

Neil

--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

This is all an indication of our increasing usage. Several PostgreSQL
articles have appeared in the past week in _major_ media outlets, not
just the open-source press (eweek, Bloomburg), a major PostgreSQL
support company bought a Linux distribution (SRA-Turbolinux), and we
have independent projects auditing our code. It is only going to get
worse. ;-)

---------------------------------------------------------------------------

Vince Vielhaber wrote:

On 20 Aug 2002, Neil Conway wrote:

Vince Vielhaber <vev@michvhf.com> writes:

Here's yet another.

Should someone from the core team perhaps get in contact with this guy
and ask if he could get in contact with the development team before
publicizing any further security holes? AFAIK that is standard
operating procedure in most cases...

I just asked him/her that on bugtraq.

Vince.
--
==========================================================================
Vince Vielhaber -- KA8CSH email: vev@michvhf.com http://www.pop4.net
56K Nationwide Dialup from $16.00/mo at Pop4 Networking
http://www.camping-usa.com http://www.cloudninegifts.com
http://www.meanstreamradio.com http://www.unknown-artists.com
==========================================================================

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  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, Pennsylvania 19073

Should someone from the core team perhaps get in contact with this guy
and ask if he could get in contact with the development team before
publicizing any further security holes? AFAIK that is standard
operating procedure in most cases...

Second, it might be worth pushing a 7.2.2 release containing the fix
for this bug, as well as the datetime problem. If that sounds
reasonable to the people who have to do the most work on a new release
(e.g. Marc), I can volunteer to backport a fix for the datetime
problem.

It'd be better to contact the dude and get all his bugs out of him, fix them
all and issue a 7.2.2 with all the fixes.

I think this is now essential - people will be using 7.2 series for ages
even after the 7.3 release...

Chris

On Wed, 21 Aug 2002, Christopher Kings-Lynne wrote:

Should someone from the core team perhaps get in contact with this guy
and ask if he could get in contact with the development team before
publicizing any further security holes? AFAIK that is standard
operating procedure in most cases...

Second, it might be worth pushing a 7.2.2 release containing the fix
for this bug, as well as the datetime problem. If that sounds
reasonable to the people who have to do the most work on a new release
(e.g. Marc), I can volunteer to backport a fix for the datetime
problem.

It'd be better to contact the dude and get all his bugs out of him, fix them
all and issue a 7.2.2 with all the fixes.

That wouldn't work because it seems he is making advisories at the
time he discovers a bug/flaw. That is, he is not directly interested in
the robustness of Postgres -- rather, another poster put it, his
reputation on bugtraq. That's fine but it doesn't mesh well with the
co-ordinated effort you describe.

I still do not see this as being a serious security problem unless you are
providing access to postgres to untrusted users. The advisory's
recommendation of killing the postmaster as a solution to these bugs is
akin to saying 'kill ssh if there's a libc bug'. If you are providing
access to untrusted users, that advice is worthwhile. But if your users
are trusted and could produce the same effect in any other number of
reasons, the advice is useless.

As for making a 7.2.2 release for the sake of form and for those users who
do provide access to untrusted users (universities, ISPs, say) this would
be pointless without the changes to opaque which Tom has put forward and
may/may not work on before 7.3 beta. I'm not sure that the core guys would
be too happy doing that *and* requiring an initdb for a minor release (as
I presume Tom's changes will require one).

Gavin

Gavin Sherry <swm@linuxworld.com.au> writes:

As for making a 7.2.2 release for the sake of form and for those
users who do provide access to untrusted users (universities, ISPs,
say) this would be pointless without the changes to opaque which Tom
has put forward and may/may not work on before 7.3 beta.

I don't think that releasing 7.2.2 without the opaque changes would be
pointless. For one thing, the opaque-related security problems are
comparatively minor: the cracker must be able to execute arbitrary SQL
commands against the database, and by that stage of the game, a DoS
attack is already trivial (e.g. disable GEQO and execute a 15 table
join query).

Also, from skimming the discussion on fixing the opaque problems,
there will be at least some degree of backwards incompatibility. That
is definitely undesirable for a stable point release -- as is an
initdb, as you point out. This amount of pain to fix a minor security
hole is *not* worth it, IMHO.

So I think that fixing the opaque problems in 7.2.x is simply
impossible. Given that, the question is whether we should make a 7.2.2
release with fixes for the other security holes (lpad(), rpad(),
reverse(), and the datetime overruns). IMHO, we should.

The datetime hole is fairly serious: it's not unreasonable for
developers to accept datetime input from the user without limiting
it's length. So it's quite likely that there are 7.2 systems in
production that have a sane security policy (e.g. hidden behind a
firewall, validate user input, etc.) that are still vulnerable.

The alternative seems unattractive: if we require that users wait for
7.3 to come out, it may be months before the 7.3.0 release. And even
then, upgrading to 7.3 is non-trivial: it requires an initdb and
reload, as well as testing to ensure that the user's applications run
smoothly on 7.3. Therefore, it may be several months before many
production sites upgrade to 7.3; leaving them in the cold for that
period isn't something I think we should do, if we can avoid it.

That said, there's only a limited amount that I can do. I think we
should make a 7.2.2 release, and to that end I've posted patches
against REL7_2_STABLE for all four of the security holes. The rest of
the work that goes into making a release needs to be done by others
(Marc, Vince, Bruce, etc.) -- if there's anything I can do to help,
let me know.

Cheers,

Neil

--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

...

So I think that fixing the opaque problems in 7.2.x is simply
impossible. Given that, the question is whether we should make a 7.2.2
release with fixes for the other security holes (lpad(), rpad(),
reverse(), and the datetime overruns). IMHO, we should.

Just a minor point: can someone actually show a symptom with date/time
problems in 7.2.x?

- Thomas

On Tue, 20 Aug 2002, Thomas Lockhart wrote:

...

So I think that fixing the opaque problems in 7.2.x is simply
impossible. Given that, the question is whether we should make a 7.2.2
release with fixes for the other security holes (lpad(), rpad(),
reverse(), and the datetime overruns). IMHO, we should.

Just a minor point: can someone actually show a symptom with date/time
problems in 7.2.x?

template1=# select version();
version
-------------------------------------------------------------
PostgreSQL 7.2.1 on i686-pc-linux-gnu, compiled by GCC 2.96
(1 row)

template1=# select
'1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111'::date
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!#

ParseDateTime() isn't checking that str < MAXDATELEN -- which is the
problem you solved in the datetime.c fixes.

Gavin

On 20 Aug 2002, Neil Conway wrote:

Vince Vielhaber <vev@michvhf.com> writes:

Here's yet another.

Should someone from the core team perhaps get in contact with this guy
and ask if he could get in contact with the development team before
publicizing any further security holes? AFAIK that is standard
operating procedure in most cases...

Second, it might be worth pushing a 7.2.2 release containing the fix
for this bug, as well as the datetime problem. If that sounds
reasonable to the people who have to do the most work on a new release
(e.g. Marc), I can volunteer to backport a fix for the datetime
problem.

I have no problems doing a v7.2.2 release for something like this ...

On Tue, 2002-08-20 at 23:34, Neil Conway wrote:

Gavin Sherry <swm@linuxworld.com.au> writes:

As for making a 7.2.2 release for the sake of form and for those
users who do provide access to untrusted users (universities, ISPs,
say) this would be pointless without the changes to opaque which Tom
has put forward and may/may not work on before 7.3 beta.

I don't think that releasing 7.2.2 without the opaque changes would be
pointless. For one thing, the opaque-related security problems are
comparatively minor: the cracker must be able to execute arbitrary SQL
commands against the database, and by that stage of the game, a DoS
attack is already trivial (e.g. disable GEQO and execute a 15 table
join query).

Also, from skimming the discussion on fixing the opaque problems,
there will be at least some degree of backwards incompatibility. That
is definitely undesirable for a stable point release -- as is an
initdb, as you point out. This amount of pain to fix a minor security
hole is *not* worth it, IMHO.

So I think that fixing the opaque problems in 7.2.x is simply
impossible. Given that, the question is whether we should make a 7.2.2
release with fixes for the other security holes (lpad(), rpad(),
reverse(), and the datetime overruns). IMHO, we should.

The datetime hole is fairly serious: it's not unreasonable for
developers to accept datetime input from the user without limiting
it's length. So it's quite likely that there are 7.2 systems in
production that have a sane security policy (e.g. hidden behind a
firewall, validate user input, etc.) that are still vulnerable.

The alternative seems unattractive: if we require that users wait for
7.3 to come out, it may be months before the 7.3.0 release. And even
then, upgrading to 7.3 is non-trivial: it requires an initdb and
reload, as well as testing to ensure that the user's applications run
smoothly on 7.3. Therefore, it may be several months before many
production sites upgrade to 7.3; leaving them in the cold for that
period isn't something I think we should do, if we can avoid it.

That said, there's only a limited amount that I can do. I think we
should make a 7.2.2 release, and to that end I've posted patches
against REL7_2_STABLE for all four of the security holes. The rest of
the work that goes into making a release needs to be done by others
(Marc, Vince, Bruce, etc.) -- if there's anything I can do to help,
let me know.

Cheers,

Neil

Assuming that we do go ahead with a 7.2.2 release, can we get some kind
of unofficial statement on pushing back the 7.3 beta? I know Tom was
hoping to start it by sept 1 but that seems rushed to me. Furthermore,
between schema support and now more backward incompatibility in regards
to functions/opaque, should we open some discussion on 7.3 really being
8.0?

Robert Treat