public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] contrib/xml2: backend crash in xpath_nodeset() on the namespace axis
4+ messages / 2 participants
[nested] [flat]
* [PATCH] contrib/xml2: backend crash in xpath_nodeset() on the namespace axis
@ 2026-06-11 00:14 Andrey Chernyy <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Andrey Chernyy @ 2026-06-11 00:14 UTC (permalink / raw)
To: [email protected]; +Cc: Michael Paquier <[email protected]>
Hi,
In the xml2 extension, xpath_nodeset() crashes the backend on an XPath
that selects namespace-axis nodes. xpath_nodeset() is executable by
PUBLIC by default, so any role that can run SQL can crash the server
with one query:
CREATE EXTENSION xml2;
SELECT xpath_nodeset('<root
xmlns:foo="http://example.com/foo"><child/></root>';,
'//namespace::*');
Cause: pgxmlNodeSetToText() (contrib/xml2/xpath.c:197) calls
xmlNodeDump(buf, nodeset->nodeTab[i]->doc, nodeset->nodeTab[i], 1,
0);
with no node-type check. Namespace-axis results are XML_NAMESPACE_DECL
nodes (xmlNs structs cast to xmlNodePtr), so reading the node's ->doc
field runs past the smaller xmlNs allocation, and the bogus value is
then dereferenced as the document by xmlNodeDump(). xpath_list() and
xpath_table() already avoid this via xmlXPathCastNodeToString(); only
the xmlNodeDump() path is exposed.
Reproduced on master; the same unguarded xmlNodeDump() call in
pgxmlNodeSetToText() is present on every supported back-branch (REL_18
through REL_14).
Patch attached: render XML_NAMESPACE_DECL nodes with
xmlXPathCastNodeToString() like xpath_table() does. The repro then
returns the namespace text, ordinary node-set output is unchanged, and
the xml2 regression test passes.
--
Andrey Chernyy
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PATCH] contrib/xml2: backend crash in xpath_nodeset() on the namespace axis
@ 2026-06-11 01:48 Michael Paquier <[email protected]>
parent: Andrey Chernyy <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Michael Paquier @ 2026-06-11 01:48 UTC (permalink / raw)
To: Andrey Chernyy <[email protected]>; +Cc: [email protected]
On Thu, Jun 11, 2026 at 03:14:36AM +0300, Andrey Chernyy wrote:
> Reproduced on master; the same unguarded xmlNodeDump() call in
> pgxmlNodeSetToText() is present on every supported back-branch (REL_18
> through REL_14).
>
> Patch attached: render XML_NAMESPACE_DECL nodes with
> xmlXPathCastNodeToString() like xpath_table() does. The repro then
> returns the namespace text, ordinary node-set output is unchanged, and
> the xml2 regression test passes.
Thanks for the report, that looks about right to fix the way you are
doing in xml2. Some tests and we should be good.
Hmm. We have a second caller of xmlNodeDump() in the core backend
code in adt/xml.c, leading to a confusing error if I try to use a
namespace:
=# select xpath('//namespace::*', '<root xmlns:foo="http://example.com"/>'::xml);
ERROR: 53200: could not copy node
CONTEXT: SQL function "xpath" statement 1
LOCATION: xml_ereport, xml.c:2082
It looks to me that we should fallback to xmlXPathCastNodeToString()
when dealing with a NAMESPACE_DECL. The attached patch leads me to
the following result, that looks much better:
=# select xpath('//namespace::*', '<root xmlns:foo="http://example.com"/>'::xml);
xpath
-----------------------------------------------------------
{http://www.w3.org/XML/1998/namespace,http://example.com}
(1 row)
What do you think? That also deserves a backpatch to me, even if it
is less worse than the xml2 crash you have reported.
I have grouped this fix with your patch in the attached. Both still
need some tests. Each fix deserves its own commit, that's just a
quick FYI version.
--
Michael
From 075d59e29cd4a1fd5595bb4fb2596f40e5a12569 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Thu, 11 Jun 2026 10:45:10 +0900
Subject: [PATCH v2] xml2: don't crash on namespace nodes in xpath_nodeset()
pgxmlNodeSetToText() passed nodeTab[i]->doc to xmlNodeDump() without
checking the node type. Namespace-axis results are XML_NAMESPACE_DECL
nodes (xmlNs structs cast to xmlNodePtr) whose ->doc field is out of
bounds, so xmlNodeDump() dereferenced a bogus pointer and crashed the
backend. Render such nodes with xmlXPathCastNodeToString() instead, as
xpath_table() already does.
---
src/backend/utils/adt/xml.c | 3 ++-
contrib/xml2/xpath.c | 19 +++++++++++++++----
2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 2c7f778cfdb7..a50992876cd6 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -4199,7 +4199,8 @@ xml_xmlnodetoxmltype(xmlNodePtr cur, PgXmlErrorContext *xmlerrcxt)
{
xmltype *result = NULL;
- if (cur->type != XML_ATTRIBUTE_NODE && cur->type != XML_TEXT_NODE)
+ if (cur->type != XML_ATTRIBUTE_NODE && cur->type != XML_TEXT_NODE &&
+ cur->type != XML_NAMESPACE_DECL)
{
void (*volatile nodefree) (xmlNodePtr) = NULL;
volatile xmlBufferPtr buf = NULL;
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index 283bb51178d1..25a1cc47577a 100644
--- a/contrib/xml2/xpath.c
+++ b/contrib/xml2/xpath.c
@@ -188,16 +188,27 @@ pgxmlNodeSetToText(xmlNodeSetPtr nodeset,
}
else
{
+ xmlNodePtr node = nodeset->nodeTab[i];
+
if ((septagname != NULL) && (xmlStrlen(septagname) > 0))
{
xmlBufferWriteChar(buf, "<");
xmlBufferWriteCHAR(buf, septagname);
xmlBufferWriteChar(buf, ">");
}
- xmlNodeDump(buf,
- nodeset->nodeTab[i]->doc,
- nodeset->nodeTab[i],
- 1, 0);
+
+ if (node->type == XML_NAMESPACE_DECL)
+ {
+ str = xmlXPathCastNodeToString(node);
+ if (str == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate node text");
+ xmlBufferWriteCHAR(buf, str);
+ xmlFree(str);
+ str = NULL;
+ }
+ else
+ xmlNodeDump(buf, node->doc, node, 1, 0);
if ((septagname != NULL) && (xmlStrlen(septagname) > 0))
{
--
2.54.0
Attachments:
[text/plain] v2-0001-xml2-don-t-crash-on-namespace-nodes-in-xpath_node.patch (2.4K, ../../[email protected]/2-v2-0001-xml2-don-t-crash-on-namespace-nodes-in-xpath_node.patch)
download | inline diff:
From 075d59e29cd4a1fd5595bb4fb2596f40e5a12569 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Thu, 11 Jun 2026 10:45:10 +0900
Subject: [PATCH v2] xml2: don't crash on namespace nodes in xpath_nodeset()
pgxmlNodeSetToText() passed nodeTab[i]->doc to xmlNodeDump() without
checking the node type. Namespace-axis results are XML_NAMESPACE_DECL
nodes (xmlNs structs cast to xmlNodePtr) whose ->doc field is out of
bounds, so xmlNodeDump() dereferenced a bogus pointer and crashed the
backend. Render such nodes with xmlXPathCastNodeToString() instead, as
xpath_table() already does.
---
src/backend/utils/adt/xml.c | 3 ++-
contrib/xml2/xpath.c | 19 +++++++++++++++----
2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 2c7f778cfdb7..a50992876cd6 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -4199,7 +4199,8 @@ xml_xmlnodetoxmltype(xmlNodePtr cur, PgXmlErrorContext *xmlerrcxt)
{
xmltype *result = NULL;
- if (cur->type != XML_ATTRIBUTE_NODE && cur->type != XML_TEXT_NODE)
+ if (cur->type != XML_ATTRIBUTE_NODE && cur->type != XML_TEXT_NODE &&
+ cur->type != XML_NAMESPACE_DECL)
{
void (*volatile nodefree) (xmlNodePtr) = NULL;
volatile xmlBufferPtr buf = NULL;
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index 283bb51178d1..25a1cc47577a 100644
--- a/contrib/xml2/xpath.c
+++ b/contrib/xml2/xpath.c
@@ -188,16 +188,27 @@ pgxmlNodeSetToText(xmlNodeSetPtr nodeset,
}
else
{
+ xmlNodePtr node = nodeset->nodeTab[i];
+
if ((septagname != NULL) && (xmlStrlen(septagname) > 0))
{
xmlBufferWriteChar(buf, "<");
xmlBufferWriteCHAR(buf, septagname);
xmlBufferWriteChar(buf, ">");
}
- xmlNodeDump(buf,
- nodeset->nodeTab[i]->doc,
- nodeset->nodeTab[i],
- 1, 0);
+
+ if (node->type == XML_NAMESPACE_DECL)
+ {
+ str = xmlXPathCastNodeToString(node);
+ if (str == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate node text");
+ xmlBufferWriteCHAR(buf, str);
+ xmlFree(str);
+ str = NULL;
+ }
+ else
+ xmlNodeDump(buf, node->doc, node, 1, 0);
if ((septagname != NULL) && (xmlStrlen(septagname) > 0))
{
--
2.54.0
[application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PATCH] contrib/xml2: backend crash in xpath_nodeset() on the namespace axis
@ 2026-06-11 22:59 Andrey Chernyy <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Andrey Chernyy @ 2026-06-11 22:59 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: [email protected]
On Thu, 11 Jun 2026 10:48:30 +0900
Michael Paquier <[email protected]> wrote:
> What do you think? That also deserves a backpatch to me, even if it
> is less worse than the xml2 crash you have reported.
>
> I have grouped this fix with your patch in the attached. Both still
> need some tests. Each fix deserves its own commit, that's just a
> quick FYI version.
Thanks Michael, and thanks for adding the tests and back-patching! The
grouped patch looks good to me - applying the same XML_NAMESPACE_DECL
fallback to xmlXPathCastNodeToString() in core xml.c makes sense too. I
had a regression test in progress, but glad it's already covered.
--
Andrey Chernyy
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PATCH] contrib/xml2: backend crash in xpath_nodeset() on the namespace axis
@ 2026-06-12 01:27 Michael Paquier <[email protected]>
parent: Andrey Chernyy <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Michael Paquier @ 2026-06-12 01:27 UTC (permalink / raw)
To: Andrey Chernyy <[email protected]>; +Cc: [email protected]
On Fri, Jun 12, 2026 at 01:59:06AM +0300, Andrey Chernyy wrote:
> Thanks Michael, and thanks for adding the tests and back-patching! The
> grouped patch looks good to me - applying the same XML_NAMESPACE_DECL
> fallback to xmlXPathCastNodeToString() in core xml.c makes sense too. I
> had a regression test in progress, but glad it's already covered.
Both issues are now fixed and backpatched, as of 8bf257aebac1 and
9d33a5a804db.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-06-12 01:27 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-06-11 00:14 [PATCH] contrib/xml2: backend crash in xpath_nodeset() on the namespace axis Andrey Chernyy <[email protected]>
2026-06-11 01:48 ` Michael Paquier <[email protected]>
2026-06-11 22:59 ` Andrey Chernyy <[email protected]>
2026-06-12 01:27 ` Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox