Обсуждение: BUG #4015: uninitialized value passed as an argument to tm2timetz

The following bug has been logged online:

Bug reference:      4015
Logged by:          Ted Kremenek
Email address:      kremenek@apple.com
PostgreSQL version: 8.3.0
Operating system:   Mac OS X 10.5.2
Description:        uninitialized value passed as an argument to tm2timetz
Details:

It appears that there may be a case where the function tm2timetz is called
with an uninitialized (i.e., undefined) value passed as one of its arguments
(actually multiple arguments are undefined).  This appears to occur along an
error path when the  date string is not properly formatted.  I'm not certain
if this would be a real bug in practice, but it looks like a classic case
where an input processing function fails to properly recover from malformed
input.

Here is the code and diagnosis.

In postgresql-8.3.0/src/backend/utils/adt/date.c:


Datum
timetz_in(PG_FUNCTION_ARGS)
{
... <SNIP>
    int            tz;
... <SNIP>

    dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
                          field, ftype, MAXDATEFIELDS, &nf);

==> In the case ParseDateTime fails, dterr has a non-zero value.

    if (dterr == 0) {
         ==> This branch is NOT taken.
         ==> This is the only branch that initializes "tz" by passing it
         ==> by reference to DecodeTimeOnly.  Note that the struct
         ==> referred to by tm is also not initialized.

       dterr = DecodeTimeOnly(field, ftype, nf, &dtype, tm, &fsec, &tz);
       }

    if (dterr != 0) {

            ==> This branch is TAKEN.
            ==> Note that "tz" is not initialized, nor does
            ==> DateTimeParseError cause execution to abort.

            DateTimeParseError(dterr, str, "time with time zone");
         }

    result = (TimeTzADT *) palloc(sizeof(TimeTzADT));

==> ERROR:
==> At this point "tz" is passed to tm2timetz, which is uninitialized.
==> Note that the value pointed to be "tm" is also uninitialized,
==> meaning that tm2timetz is passed garbage that sometimes
==> may be valid (due to remnants of data still on the stack).

    tm2timetz(tm, fsec, tz, result);
    AdjustTimeForTypmod(&(result->time), typmod);

    PG_RETURN_TIMETZADT_P(result);
}

"Ted Kremenek" <kremenek@apple.com> writes:
> It appears that there may be a case where the function tm2timetz is called
> with an uninitialized (i.e., undefined) value passed as one of its arguments
> (actually multiple arguments are undefined).

I think you're assuming DateTimeParseError() will return, which it will
not (it always throws ereport(ERROR)).  Or have I missed something?

            regards, tom lane

On Mar 5, 2008, at 10:10 PM, Tom Lane wrote:

> "Ted Kremenek" <kremenek@apple.com> writes:
>> It appears that there may be a case where the function tm2timetz is
>> called
>> with an uninitialized (i.e., undefined) value passed as one of its
>> arguments
>> (actually multiple arguments are undefined).
>
> I think you're assuming DateTimeParseError() will return, which it
> will
> not (it always throws ereport(ERROR)).  Or have I missed something?
>
>             regards, tom lane

Thanks Tom.

After your comments I looked more closely at ereport.  It's a little
buried, but I can see that when errstart (wrapped by ereport) is
called with ERROR that it does not return.

Thanks for the quick response.

Ted