Re: No sanity checking performed on binary TIME parameters.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Andrew McNamara <andrewm(at)object-craft(dot)com(dot)au>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: No sanity checking performed on binary TIME parameters.
Date: 2009-05-25 19:41:10
Message-ID: 6921.1243280470@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> I'm not entirely sure why we put a range limit on time values at all,
>> but given that we do, it'd probably be a good idea to check the range
>> in the recv functions. I'm inclined to fix this for 8.4, but not
>> back-patch because of compatibility considerations. Any objections
>> out there?

> Are we confident it can't be abused to impact other clients connecting
> or break the back-end in some way? More specifically, could it be a
> security issue? Havn't looked at it yet, but getting what sounded like
> corrupted data back out could be bad..

The only place I can find where an oversize time value behaves in a
seriously bogus fashion is in time_out, or more specifically
EncodeTimeOnly(): it fails to initialize its output string at all.
So you could easily get garbage text output, though in my quick tests
you seem to usually get an empty string instead. The odds of an actual
crash seem pretty small, but not quite zero (if somehow there was no
zero byte up to the end of the stack).

My feeling is that the error check in EncodeTimeOnly is just stupid and
should be removed. That code will work fine with oversize times (and
no, it won't overrun the output buffers either). The callers aren't
bothering to check for error returns anyway...

On the whole the argument that it could be a security problem seems
pretty thin.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2009-05-25 20:53:12 Re: generic options for explain
Previous Message Tom Lane 2009-05-25 19:02:47 Re: A couple of gripes about the gettext plurals patch