[HACKERS] small fix to possible null pointer dereference in byteaout() varlena.c

2010-09-28 Thread Grzegorz Jaśkiewicz
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
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 363fd3c..48ee55d 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -351,6 +351,7 @@ byteaout(PG_FUNCTION_ARGS)
 			else
 *rp++ = *vp;
 		}
+*rp = '\0';
 	}
 	else
 	{
@@ -358,7 +359,7 @@ byteaout(PG_FUNCTION_ARGS)
 			 bytea_output);
 		rp = result = NULL;		/* keep compiler quiet */
 	}
-	*rp = '\0';
+
 	PG_RETURN_CSTRING(result);
 }
 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2010-09-28 Thread Tom Lane
=?UTF-8?Q?Grzegorz_Ja=C5=9Bkiewicz?= gryz...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2010-09-28 Thread Grzegorz Jaśkiewicz
2010/9/28 Tom Lane t...@sss.pgh.pa.us:
 =?UTF-8?Q?Grzegorz_Ja=C5=9Bkiewicz?= gryz...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2010-09-28 Thread Tom Lane
=?UTF-8?Q?Grzegorz_Ja=C5=9Bkiewicz?= gryz...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2010-09-28 Thread Robert Haas
2010/9/28 Grzegorz Jaśkiewicz gryz...@gmail.com:
 2010/9/28 Tom Lane t...@sss.pgh.pa.us:
 =?UTF-8?Q?Grzegorz_Ja=C5=9Bkiewicz?= gryz...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2010-09-28 Thread Grzegorz Jaśkiewicz
got it folks. Forgot that elog doesn't return with ERROR

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers