Thread: BUG #15624: Sefgault when xml_errorHandler receives a null error->message from libxml2
BUG #15624: Sefgault when xml_errorHandler receives a null error->message from libxml2
From
PG Bug reporting form
Date:
The following bug has been logged on the website: Bug reference: 15624 Logged by: Sergio Conde Gómez Email address: skgsergio@gmail.com PostgreSQL version: 10.6 Operating system: Ubuntu Linux 16.04.5 LTS (Xenial Xerus) Description: Hello, We've got a segfault when xml_errorHandler called appendStringInfoString with null error->message, this ends calling strlen(NULL). This is the struct received by xml_errorHandler was the following: (gdb) print *error $1 = {domain = 12, code = 2, message = 0x0, level = XML_ERR_FATAL, file = 0x0, line = 0, str1 = 0x5643cf615fe0 "creating context\n", str2 = 0x0, str3 = 0x0, int1 = 0, int2 = 0, ctxt = 0x0, node = 0x0} According to libxml2 (we are using v2.9.2) domain 12 is XML_FROM_XPATH and code 2 is XML_ERR_NO_MEMORY so postgre's xml_errorHandler it will try to append the message. Although libxml2 tries not to return a null message but both their xmlStrdup function and XML_GET_VAR_STR can return null in a OOM scenario. This also affects PostgreSQL 11 branch so here it is the proposed patch both for REL_10_STABLE and REL_11_STABLE: --- src/backend/utils/adt/xml.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c index 37d85f71f3..3b36544987 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -1696,7 +1696,8 @@ xml_errorHandler(void *data, xmlErrorPtr error) appendStringInfo(errorBuf, "line %d: ", error->line); if (name != NULL) appendStringInfo(errorBuf, "element %s: ", name); - appendStringInfoString(errorBuf, error->message); + if (error->message != NULL) + appendStringInfoString(errorBuf, error->message); /* * Append context information to errorBuf. -- 2.20.1
Re: BUG #15624: Sefgault when xml_errorHandler receives a null error->message from libxml2
From
Tom Lane
Date:
PG Bug reporting form <noreply@postgresql.org> writes: > Although libxml2 tries not to return a null message but both their xmlStrdup > function and XML_GET_VAR_STR can return null in a OOM scenario. Ugh. > - appendStringInfoString(errorBuf, error->message); > + if (error->message != NULL) > + appendStringInfoString(errorBuf, error->message); I'm inclined to do something more like + if (error->message != NULL) + appendStringInfoString(errorBuf, error->message); + else + appendStringInfoString(errorBuf, "(no message provided)"); else the output will read very oddly in this situation. Thanks for the report! regards, tom lane
Re: BUG #15624: Sefgault when xml_errorHandler receives a nullerror->message from libxml2
From
Sergio Conde Gómez
Date:
Yes, you are right. Didn't really checked the full output so it makes sense to do that to be consistent as almost always there will be a message. Thanks!
El vie., 8 feb. 2019 a las 18:53, Tom Lane (<tgl@sss.pgh.pa.us>) escribió:
PG Bug reporting form <noreply@postgresql.org> writes:
> Although libxml2 tries not to return a null message but both their xmlStrdup
> function and XML_GET_VAR_STR can return null in a OOM scenario.
Ugh.
> - appendStringInfoString(errorBuf, error->message);
> + if (error->message != NULL)
> + appendStringInfoString(errorBuf, error->message);
I'm inclined to do something more like
+ if (error->message != NULL)
+ appendStringInfoString(errorBuf, error->message);
+ else
+ appendStringInfoString(errorBuf, "(no message provided)");
else the output will read very oddly in this situation.
Thanks for the report!
regards, tom lane
Sergio Conde
GPG Key: 0x1867A20A
Fingerprint: 487D 62C8 523C 9BBF 7CC8 D029 959E A15D 1867 A20A
http://keybase.io/skgsergio
GPG Key: 0x1867A20A
Fingerprint: 487D 62C8 523C 9BBF 7CC8 D029 959E A15D 1867 A20A
http://keybase.io/skgsergio
Re: BUG #15624: Sefgault when xml_errorHandler receives a null error->message from libxml2
From
Tom Lane
Date:
=?UTF-8?Q?Sergio_Conde_G=C3=B3mez?= <skgsergio@gmail.com> writes: > Yes, you are right. Didn't really checked the full output so it makes sense > to do that to be consistent as almost always there will be a message. > Thanks! Pushed with that change, will be in next week's releases. regards, tom lane