Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 12 Mar 2026 15:46:57 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: cca5507 <cca5507@qq.com>, Jim Jones <jim.jones@uni-muenster.de>,
	Tom Lane <tgl@sss.pgh.pa.us>,
	pgsql-bugs <pgsql-bugs@lists.postgresql.org>,
	maralist86 <maralist86@mail.ru>
Subject: Re: BUG #18943: Return value of a function 'xmlBufferCreate'
 isdereferenced at xpath.c:177 without checking for NUL
Message-ID: <abJhYZ7YIlXm8IZo@paquier.xyz>
References: <d2410ca0-c0dd-4f63-9e70-3d7a62a5d705@uni-muenster.de>
 <b35e2342-0f02-4365-94cf-55052ac9bda1@uni-muenster.de>
 <aEKCoNIfLxjyKY3r@paquier.xyz>
 <31f3480e-cd7d-4021-b392-87922572cc37@uni-muenster.de>
 <aETzMep2fGfB0AIp@paquier.xyz>
 <tencent_5BE8DAD985EE140ED62EA728C8D4E1311F0A@qq.com>
 <tencent_51B1563E654EA6188FF240C8CE2434B6B005@qq.com>
 <aa53WWdR7E4z345v@paquier.xyz>
 <aa9FRUsCenN8Vfum@paquier.xyz>
 <c516a0d9-4406-47e3-9087-5ca5176ebcf9@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="DkMJxZxGvYwgaEsz"
Content-Disposition: inline
In-Reply-To: <c516a0d9-4406-47e3-9087-5ca5176ebcf9@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/abJhYZ7YIlXm8IZo%40paquier.xyz>
Precedence: bulk


--DkMJxZxGvYwgaEsz
Content-Type: multipart/mixed; boundary="agExfr1MrcV4Zryl"
Content-Disposition: inline


--agExfr1MrcV4Zryl
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Mar 12, 2026 at 07:00:00AM +0200, Alexander Lakhin wrote:
> Hello Michael,
>=20
> Maybe you would like to fix in passing one more anomaly there:
> create extension xml2;
> select xslt_process('<aaa/>','<xsl:stylesheet version=3D"1.0"
> xmlns:xsl=3D"http://www.w3.org/1999/XSL/Transform"></xsl:stylesheet>');
>=20
> leads to:
> varlena.c:199:2: runtime error: null pointer passed as argument 2, which =
is declared to never be null
> =A0=A0=A0 #0 0x640756666936 in cstring_to_text_with_len .../src/backend/u=
tils/adt/varlena.c:199
> =A0=A0=A0 #1 0x7e46c0f4805e in xslt_process .../contrib/xml2/xslt_proc.c:=
149
> =A0=A0=A0 #2 0x640755a3ecbf in ExecInterpExpr .../src/backend/executor/ex=
ecExprInterp.c:1001
> =A0=A0=A0 #3 0x640755a277aa in ExecInterpExprStillValid .../src/backend/e=
xecutor/execExprInterp.c:2299
> =A0=A0=A0 #4 0x640755ef11e0 in ExecEvalExprSwitchContext ../../../../src/=
include/executor/executor.h:444
> =A0=A0=A0 #5 0x640755efd7b6 in evaluate_expr .../src/backend/optimizer/ut=
il/clauses.c:5724
>=20
> for a build made with -fsanitize=3Dundefined.

Indeed, I can reproduce it locally.  This one is a super old
inconsistency, from what I can see.  This predates the introduction to
xml2 in contrib and even the use of cstring_to_text_with_len().  We've
never thought that xsltSaveResultToString() could return a NULL
xmlChar with a valid status code and a length of 0.  Back in the
day, before cstring_to_text_with_len(), that would be a memcpy with a
NULL pointer.

I am not sure if this is worth backpatching, so let's just use
something like the attached on HEAD.  This result cannot be NULL,
historically it has always been an empty string.

Opinions?
--
Michael

--agExfr1MrcV4Zryl
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=0001-xml2-Fix-undeterministic-result-with-xslt_process.patch
Content-Transfer-Encoding: quoted-printable

=46rom 6eb8518de5c3d767bb6b58426bb04b2173166f2c Mon Sep 17 00:00:00 2001
=46rom: Michael Paquier <michael@paquier.xyz>
Date: Thu, 12 Mar 2026 15:43:45 +0900
Subject: [PATCH] xml2: Fix undeterministic result with xslt_process()

---
 contrib/xml2/expected/xml2.out   | 10 ++++++++++
 contrib/xml2/expected/xml2_1.out |  6 ++++++
 contrib/xml2/sql/xml2.sql        |  6 ++++++
 contrib/xml2/xslt_proc.c         |  8 +++++++-
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/contrib/xml2/expected/xml2.out b/contrib/xml2/expected/xml2.out
index 3d97b14c3a1e..1906fcf33e2a 100644
--- a/contrib/xml2/expected/xml2.out
+++ b/contrib/xml2/expected/xml2.out
@@ -261,3 +261,13 @@ $$<xsl:stylesheet version=3D"1.0"
   </xsl:template>
 </xsl:stylesheet>$$);
 ERROR:  failed to apply stylesheet
+-- empty output
+select xslt_process('<aaa/>',
+$$<xsl:stylesheet version=3D"1.0"
+      xmlns:xsl=3D"http://www.w3.org/1999/XSL/Transform">
+</xsl:stylesheet>$$);
+ xslt_process=20
+--------------
+=20
+(1 row)
+
diff --git a/contrib/xml2/expected/xml2_1.out b/contrib/xml2/expected/xml2_=
1.out
index 31700040a604..9a2144d58f57 100644
--- a/contrib/xml2/expected/xml2_1.out
+++ b/contrib/xml2/expected/xml2_1.out
@@ -205,3 +205,9 @@ $$<xsl:stylesheet version=3D"1.0"
   </xsl:template>
 </xsl:stylesheet>$$);
 ERROR:  xslt_process() is not available without libxslt
+-- empty output
+select xslt_process('<aaa/>',
+$$<xsl:stylesheet version=3D"1.0"
+      xmlns:xsl=3D"http://www.w3.org/1999/XSL/Transform">
+</xsl:stylesheet>$$);
+ERROR:  xslt_process() is not available without libxslt
diff --git a/contrib/xml2/sql/xml2.sql b/contrib/xml2/sql/xml2.sql
index ef99d164f272..510d18a36799 100644
--- a/contrib/xml2/sql/xml2.sql
+++ b/contrib/xml2/sql/xml2.sql
@@ -153,3 +153,9 @@ $$<xsl:stylesheet version=3D"1.0"
     </sax:output>
   </xsl:template>
 </xsl:stylesheet>$$);
+
+-- empty output
+select xslt_process('<aaa/>',
+$$<xsl:stylesheet version=3D"1.0"
+      xmlns:xsl=3D"http://www.w3.org/1999/XSL/Transform">
+</xsl:stylesheet>$$);
diff --git a/contrib/xml2/xslt_proc.c b/contrib/xml2/xslt_proc.c
index 2be87bec0cdf..23e5509b99ad 100644
--- a/contrib/xml2/xslt_proc.c
+++ b/contrib/xml2/xslt_proc.c
@@ -145,7 +145,13 @@ xslt_process(PG_FUNCTION_ARGS)
 		resstat =3D xsltSaveResultToString((xmlChar **) &resstr, &reslen,
 										 restree, stylesheet);
=20
-		if (resstat >=3D 0)
+		/*
+		 * If an empty string has been returned, resstr would be NULL.
+		 * In this case, assume that the result is an empty string.
+		 */
+		if (reslen =3D=3D 0)
+			result =3D cstring_to_text_with_len("", reslen);
+		else if (resstat >=3D 0)
 			result =3D cstring_to_text_with_len((char *) resstr, reslen);
 	}
 	PG_CATCH();
--=20
2.53.0


--agExfr1MrcV4Zryl--

--DkMJxZxGvYwgaEsz
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmmyYWEACgkQnvQgOdby
QH19BA//QzRd44Zn52hbfXp6lKqhyfnrpuFCNJEBYTITvF1lTzUq0FNjdImhwzhF
sWdFPnZoBDiCR+6S35lnAFs9JYXKdf2fyCoLgSrnVcxYACHY/95rpTBqJMtpkwDq
2P0VcD+eywBSkhaAjUI7c5c+YA8RbvCaBr0n5e6wSaj++6ckpQU0ytXKyXr9HlSX
t+rO9J+0BwWNUsw+DjCueiCJ3yo2cLlQTYsV+d4Ps+IFk5q26ap6vojYchwAp3rc
ef34oWadSvpQLcMmCEMSJpTsW8H7NKdYvl6HcCwwZT5uMOkxRpWkpluGfXhNcTLa
g/PtWFDzEm1A818hrreMH1etUtyixt2jZqzmQ/t/RJz5RBu0hErLhCowV7je9AhG
h2/ZAVG8cGS/J8iLkO5YSU5vlD9awhLe3n1bq0blJUkixiOiAMNrEuSlNZYPb8Vh
qamLUU2LHx0jAtP6fw/RKReqtJ2DZGaeMbOnCI0pOuBZLL66bBF82Tbu/7zWG8gR
Fo+vhAI+m4ClxRKA0H2hpYxSX6j+0VApJeto6G/zL8P2Y1HKNyuO5BELjXmXRW7B
eH6GN9Xeo6Ljq5DDvMt4SBXXhgtQiJWeIEAE6dZsbmYXXc0Z5mORyO0qOLN82veC
EHthETaW6/vo0PioDFO85LJqrUL2WwfKpsGBo5omvsZvi/Yu1IE=
=yclF
-----END PGP SIGNATURE-----

--DkMJxZxGvYwgaEsz--