public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Michael Paquier <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #18943: Return value of a function 'xmlBufferCreate' is dereferenced at xpath.c:177 without checking for NUL
Date: Tue, 03 Jun 2025 13:15:33 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>

Michael Paquier <[email protected]> writes:
> I'll try to have a second look at this patch in a couple of weeks.
> For now, I have added it to the next comment fest:
> https://commitfest.postgresql.org/patch/5794/
> If somebody would like to chime in, feel free.

Thanks for taking this on --- contrib/xml2 is really a mess so far as
error handling goes.  Your patch looks like an improvement, although
I do have one concern: the routines in xml.c that use an xmlerrcxt
seem to check xmlerrcxt->err_occurred pretty frequently, eg xmlStrdup
is used like this:

            doc->encoding = xmlStrdup((const xmlChar *) "UTF-8");
            if (doc->encoding == NULL || xmlerrcxt->err_occurred)
                xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
                            "could not allocate XML document");

Not sure if that's needed here.

There's more that could be looked at, if you feel like it:

xml_encode_special_chars seems to need a PG_TRY block to avoid
possibly leaking "tt".  I also wonder if it's really not possible
for xmlEncodeSpecialChars to fail and return NULL.  (xmltext()
doesn't believe that either, but maybe it's wrong too.)

The usage of cleanup_workspace seems quite a mess: one caller
uses a PG_TRY block to ensure it's called, but the rest don't.
I also find it confusing that pgxml_xpath returns a value that is
also referenced in the workspace and cleanup_workspace is responsible
for freeing.  I wonder if there's a better way to do that.

In the end of xslt_process, I wonder why

	if (resstr)
		xmlFree((xmlChar *) resstr);

isn't done before the pg_xml_done call.

			regards, tom lane

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: BUG #18943: Return value of a function 'xmlBufferCreate' is dereferenced at xpath.c:177 without checking for NUL
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox