small fix to possible null pointer dereference in byteaout() varlena.c
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
Attachments:
varlena_null_dereference.patchtext/x-patch; charset=US-ASCII; name=varlena_null_dereference.patchDownload
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);
}
=?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
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 inI'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
=?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
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 inI'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