Horton

Thread

  1. small fix to possible null pointer dereference in byteaout() varlena.c

    Greg Jaskiewicz <gryzman@gmail.com> — 2010-09-28T13:31:48Z

    It would crash if input is of unrecognized format. Probably than
there's going to be more problems to be concerned with, but just in
case, don't crash in

-- 
GJ

  2. Re: small fix to possible null pointer dereference in byteaout() varlena.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-09-28T15:07:42Z

    =?UTF-8?Q?Grzegorz_Ja=C5=9Bkiewicz?= <gryzman@gmail.com> writes:
> It would crash if input is of unrecognized format. Probably than
> there's going to be more problems to be concerned with, but just in
> case, don't crash in

I'm not sure why you think this is a good change, but it would break
things: in particular, the code would fail to null-terminate the string
in the hex-output case.  Also, the case that you seem to be trying to
defend against can't happen because elog(ERROR) doesn't return.

			regards, tom lane

  3. Re: small fix to possible null pointer dereference in byteaout() varlena.c

    Greg Jaskiewicz <gryzman@gmail.com> — 2010-09-28T15:15:13Z

    2010/9/28 Tom Lane <tgl@sss.pgh.pa.us>:
> =?UTF-8?Q?Grzegorz_Ja=C5=9Bkiewicz?= <gryzman@gmail.com> writes:
>> It would crash if input is of unrecognized format. Probably than
>> there's going to be more problems to be concerned with, but just in
>> case, don't crash in
>
> I'm not sure why you think this is a good change, but it would break
> things: in particular, the code would fail to null-terminate the string
> in the hex-output case.  Also, the case that you seem to be trying to
> defend against can't happen because elog(ERROR) doesn't return.
>

...
                rp = result = NULL;             /* keep compiler quiet */
        }
        *rp = '\0';
....

this strikes me as a clear case of possible null pointer dereference,
wouldn't you agree ?
I know the case is very corner-ish, but still valid imo.




-- 
GJ

  4. Re: small fix to possible null pointer dereference in byteaout() varlena.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-09-28T15:23:35Z

    =?UTF-8?Q?Grzegorz_Ja=C5=9Bkiewicz?= <gryzman@gmail.com> writes:
> ...
>                 rp = result = NULL;             /* keep compiler quiet */
>         }
>         *rp = '\0';
> ....

> this strikes me as a clear case of possible null pointer dereference,
> wouldn't you agree ?

No, I wouldn't.  You need to enlarge your peephole by one line:

    else
    {
        elog(ERROR, "unrecognized bytea_output setting: %d",
             bytea_output);
        rp = result = NULL;        /* keep compiler quiet */
    }
    *rp = '\0';

The "keep compiler quiet" line is unreachable code (and that comment is
supposed to remind you of that).

			regards, tom lane

  5. Re: small fix to possible null pointer dereference in byteaout() varlena.c

    Robert Haas <robertmhaas@gmail.com> — 2010-09-28T15:24:24Z

    2010/9/28 Grzegorz Jaśkiewicz <gryzman@gmail.com>:
> 2010/9/28 Tom Lane <tgl@sss.pgh.pa.us>:
>> =?UTF-8?Q?Grzegorz_Ja=C5=9Bkiewicz?= <gryzman@gmail.com> writes:
>>> It would crash if input is of unrecognized format. Probably than
>>> there's going to be more problems to be concerned with, but just in
>>> case, don't crash in
>>
>> I'm not sure why you think this is a good change, but it would break
>> things: in particular, the code would fail to null-terminate the string
>> in the hex-output case.  Also, the case that you seem to be trying to
>> defend against can't happen because elog(ERROR) doesn't return.
>>
>
> ...
>                rp = result = NULL;             /* keep compiler quiet */
>        }
>        *rp = '\0';
> ....
>
> this strikes me as a clear case of possible null pointer dereference,
> wouldn't you agree ?
> I know the case is very corner-ish, but still valid imo.

That comment that says "keep compiler quiet" is there because we
expect that line of code never to get executed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

  6. Re: small fix to possible null pointer dereference in byteaout() varlena.c

    Greg Jaskiewicz <gryzman@gmail.com> — 2010-09-28T15:27:57Z

    got it folks. Forgot that elog doesn't return with ERROR