From: Tom Lane <tgl@sss.pgh.pa.us>
To: exclusion@gmail.com
cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17912: Invalid memory access when converting plpython' array
 containing empty array
In-reply-to: <17912-82ceed78731d9cdc@postgresql.org>
References: <17912-82ceed78731d9cdc@postgresql.org>
Comments: In-reply-to PG Bug reporting form <noreply@postgresql.org>
	message dated "Fri, 28 Apr 2023 11:00:01 -0000"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <1392959.1682694841.0@sss.pgh.pa.us>
Date: Fri, 28 Apr 2023 11:14:06 -0400
Message-ID: <1392974.1682694846@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1392974.1682694846%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1392959.1682694841.1@sss.pgh.pa.us>

PG Bug reporting form <noreply@postgresql.org> writes:
> CREATE EXTENSION plpython3u;
> CREATE OR REPLACE FUNCTION test() RETURNS text[] AS $$
> return [[], "a"]
> $$ LANGUAGE plpython3u;
> SELECT test();

> As I can see, for the first case we get len = 0 in PLySequence_ToArray();
> elems, nulls palloc'ed with zero elements, but PLyObject_ToScalar() tries
> to write a value into nulls[0]...

Yeah.  The calculation of the array size is being done in the wrong
place, so that we may update len to zero before breaking out of the
loop.  But really it's poor coding for this function to be doing
its own calculation of the array size at all, rather than consulting
the authoritative ArrayGetNItems().  I think we need something like
the attached.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-PLySequence_ToArray.patch";
	charset="us-ascii"
Content-ID: <1392959.1682694841.2@sss.pgh.pa.us>
Content-Description: fix-PLySequence_ToArray.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 7018c9d404..864b5f1765 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -1136,7 +1136,7 @@ PLySequence_ToArray(PLyObToDatum *arg, PyObject *plr=
v,
 	int			i;
 	Datum	   *elems;
 	bool	   *nulls;
-	int64		len;
+	int			len;
 	int			ndim;
 	int			dims[MAXDIM];
 	int			lbs[MAXDIM];
@@ -1155,7 +1155,6 @@ PLySequence_ToArray(PLyObToDatum *arg, PyObject *plr=
v,
 	 * Determine the number of dimensions, and their sizes.
 	 */
 	ndim =3D 0;
-	len =3D 1;
 =

 	Py_INCREF(plrv);
 =

@@ -1174,17 +1173,6 @@ PLySequence_ToArray(PLyObToDatum *arg, PyObject *pl=
rv,
 		if (dims[ndim] < 0)
 			PLy_elog(ERROR, "could not determine sequence length for function retu=
rn value");
 =

-		if (dims[ndim] > MaxAllocSize)
-			ereport(ERROR,
-					(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-					 errmsg("array size exceeds the maximum allowed")));
-
-		len *=3D dims[ndim];
-		if (len > MaxAllocSize)
-			ereport(ERROR,
-					(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-					 errmsg("array size exceeds the maximum allowed")));
-
 		if (dims[ndim] =3D=3D 0)
 		{
 			/* empty sequence */
@@ -1214,15 +1202,18 @@ PLySequence_ToArray(PLyObToDatum *arg, PyObject *p=
lrv,
 					 errmsg("return value of function with array return type is not a Py=
thon sequence")));
 =

 		ndim =3D 1;
-		len =3D dims[0] =3D PySequence_Length(plrv);
+		dims[0] =3D PySequence_Length(plrv);
 	}
 =

+	/* Allocate space for work arrays, after detecting array size overflow *=
/
+	len =3D ArrayGetNItems(ndim, dims);
+	elems =3D palloc(sizeof(Datum) * len);
+	nulls =3D palloc(sizeof(bool) * len);
+
 	/*
 	 * Traverse the Python lists, in depth-first order, and collect all the
 	 * elements at the bottom level into 'elems'/'nulls' arrays.
 	 */
-	elems =3D palloc(sizeof(Datum) * len);
-	nulls =3D palloc(sizeof(bool) * len);
 	currelem =3D 0;
 	PLySequence_ToArray_recurse(arg->u.array.elm, plrv,
 								dims, ndim, 0,

------- =_aaaaaaaaaa0--