Обсуждение: No sanity checking performed on binary TIME parameters.
When submitting a query via the V3 binary protocol (PQexecParams, paramFormats[n]=1), it appears the PostgreSQL server performs no range checking on the passed values. Passing values greater than 24 hours results in unpredictable results (dumps that cannot be restored, strange output when printing the column in psql, etc). Tested with version 8.1 and 8.2 (integer_datetimes is false). Using my python ocpgdb module (http://code.google.com/p/ocpgdb/): >>> db.execute('select %s::time::text', DateTimeDelta(0,23,59,59)) [('23:59:59',)]>>> db.execute('select %s::time::text', DateTimeDelta(0,28,0,0)) [('K|\x1f',)] ocpgdb has a lower-level API which is a thin layer on top of libpq - exercising this directly to rule out any problems with the mx.DateTime.DateTimeDelta class yields the same results: >>> import struct>>> import ocpgdb, oclibpq>>> db=oclibpq.PgConnection('')>>> list(db.execute('select $1::time::text', [(ocpgdb.pgoid.time, struct.pack('!d', 23*60*60))])) [(<PyPgCell name 'text', type 25, modifier -1, value '23:00:00' at 0x42a4a0>,)]>>> list(db.execute('select $1::time::text', [(ocpgdb.pgoid.time, struct.pack('!d', 48*60*60))])) [(<PyPgCell name 'text', type 25, modifier -1, value 'K|\x1f' at 0x42a500>,)] Apologies if this bug has already been addressed - I didn't find any references to it while googling.
Andrew McNamara <andrewm@object-craft.com.au> writes: > When submitting a query via the V3 binary protocol (PQexecParams, > paramFormats[n]=1), it appears the PostgreSQL server performs no range > checking on the passed values. A quick look at time_recv() shows this is true, and timetz_recv() checks neither the time nor the zone component. > Passing values greater than 24 hours > results in unpredictable results (dumps that cannot be restored, > strange output when printing the column in psql, etc). 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? regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.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.. Stephen
Stephen Frost <sfrost@snowman.net> writes: > * Tom Lane (tgl@sss.pgh.pa.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
On 26/05/2009, at 5:41 AM, Tom Lane wrote: > > 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). I'm seeing all sorts of odd stuff - typically the last column value output, but occasionally other snippets of random data that don't seem related to the query. > 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... I'm not sure it's postgresql's job to police things like this, but returning values greater than 24 hours may violate assumptions in user code, and I would be worried about potentially causing silent failures. Of course, it should no longer be possible to get an illegal value into the database, so the risk is low - either a database that predates the fix, or database corruption. Are there any other cases where the binary receive functions are missing sanity checks?
Andrew McNamara <andrewm@object-craft.com.au> writes: > Are there any other cases where the binary receive functions are > missing sanity checks? Possibly --- you want to go looking? regards, tom lane
On 26/05/2009, at 10:25 AM, Tom Lane wrote: > Andrew McNamara <andrewm@object-craft.com.au> writes: >> Are there any other cases where the binary receive functions are >> missing sanity checks? > > Possibly --- you want to go looking? Uh. I'd be lying if I said I "wanted to" - I got enough of a taste of those functions when trying to work out what they expect when doing my ocpgdb module. At the moment, however, I have a good excuse for wimping out - every waking hour is being expended on swine flu related work (although I'd almost welcome the distraction of a good *virtual* bug hunt).