Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1ww1lD-001hAN-1m for pgsql-hackers@arkaria.postgresql.org; Mon, 17 Aug 2026 18:08:35 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1ww1lB-00BveW-0U for pgsql-hackers@arkaria.postgresql.org; Mon, 17 Aug 2026 18:08:34 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1ww1lA-00BveM-2L for pgsql-hackers@lists.postgresql.org; Mon, 17 Aug 2026 18:08:33 +0000 Received: from sss.pgh.pa.us ([68.162.161.243]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1ww1l9-000000015m9-2bv6 for pgsql-hackers@lists.postgresql.org; Mon, 17 Aug 2026 18:08:32 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.18.1/8.18.1) with ESMTP id 67HI8S1T822137; Mon, 17 Aug 2026 14:08:28 -0400 From: Tom Lane To: Andrey Rachitskiy cc: andrew@dunslane.net, pgsql-hackers@lists.postgresql.org Subject: Re: Residual cleanups for tied objects in PL/Perl In-reply-to: <821480.1786989484@sss.pgh.pa.us> References: <569769.1786901901@sss.pgh.pa.us> <579928.1786912017@sss.pgh.pa.us> <756684.1786981296@sss.pgh.pa.us> <761068.1786985394@sss.pgh.pa.us> <821480.1786989484@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Mon, 17 Aug 2026 13:58:04 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <822101.1786990072.0@sss.pgh.pa.us> Date: Mon, 17 Aug 2026 14:08:28 -0400 Message-ID: <822136.1786990108@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <822101.1786990072.1@sss.pgh.pa.us> Looking a bit more at the issue of standardizing our perl-array-scanning code, I noticed that av_len returns SSize_t not int, creating an overflow risk since we aren't consistent about assigning the result to something wider than int. This would only result in truncating the output not anything more exciting, but it's still bad. Also, it seems from looking at the Perl sources that they prefer to use av_count() to compute the loop limit. That's the same as av_len()+1 (except it returns Size_t), but I think that's a better habit to be in since it removes the risk of forgetting to add 1. So here is a patch that just cleans up those issues. I'm inclined to push this, and separately push a patch to clean up the inconsistent hash accessing logic, and then deal with magic objects on top of that. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name*0="v2-0001-Make-plperl-s-handling-of-Perl-arrays-safer-and-m.p"; name*1="atch"; charset="us-ascii" Content-ID: <822101.1786990072.2@sss.pgh.pa.us> Content-Description: v2-0001-Make-plperl-s-handling-of-Perl-arrays-safer-and-m.patch Content-Transfer-Encoding: quoted-printable =46rom 51d0d098b82276657fcc4e8d80c87ba3cf514f98 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 17 Aug 2026 13:37:33 -0400 Subject: [PATCH v2] Make plperl's handling of Perl arrays safer and more consistent. plperl_func_handler()'s stanza for handling an arrayref result in a SETOF function could loop forever (or at least till OOM) when given a tied array, since av_fetch won't necessarily ever return a null pointer in that case. Be consistent with the other places where we traverse a perl array: call av_len() once and use len+1 as the loop limit, silently ignoring any null pointers we get back from that range of subscripts. But actually, Perl's preferred locution for this seems to be to use av_count() not av_len()+1. av_count() seems better since there's less risk of forgetting to add 1. Also, both of those functions return Size_t (or SSize_t) not int, creating at least a theoretical overflow hazard. While we're modernizing this, let's use the correct variable type where we can, and include an overflow check where we can't. Reported-by: Claude Code (via Noah Misch) Author: Tom Lane Discussion: https://postgr.es/m/569769.1786901901@sss.pgh.pa.us Backpatch-through: 14 --- contrib/jsonb_plperl/jsonb_plperl.c | 5 ++-- src/pl/plperl/plperl.c | 41 ++++++++++++++++++++++------- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/contrib/jsonb_plperl/jsonb_plperl.c b/contrib/jsonb_plperl/js= onb_plperl.c index 97d147cc65a..00a99d303c6 100644 --- a/contrib/jsonb_plperl/jsonb_plperl.c +++ b/contrib/jsonb_plperl/jsonb_plperl.c @@ -135,12 +135,11 @@ static void AV_to_JsonbValue(AV *in, JsonbInState *jsonb_state) { dTHX; - SSize_t pcount =3D av_len(in) + 1; - SSize_t i; + Size_t pcount =3D av_count(in); = pushJsonbValue(jsonb_state, WJB_BEGIN_ARRAY, NULL); = - for (i =3D 0; i < pcount; i++) + for (Size_t i =3D 0; i < pcount; i++) { SV **value =3D av_fetch(in, i, FALSE); = diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index eba91f2d7d6..8175407849e 100644 --- a/src/pl/plperl/plperl.c +++ b/src/pl/plperl/plperl.c @@ -279,6 +279,7 @@ static void array_to_datum_internal(AV *av, ArrayBuild= State **astatep, int *ndims, int *dims, int cur_depth, Oid elemtypid, int32 typmod, FmgrInfo *finfo, Oid typioparam); +static int av_count_limit(AV *av); static Datum plperl_hash_to_datum(SV *src, TupleDesc td); = static void plperl_init_shared_libs(pTHX); @@ -1173,8 +1174,8 @@ get_perl_array_ref(SV *sv) * is frozen). * * Caller is required to have set dims[cur_depth - 1] to the length of th= e - * input array, i.e., av_len(av) + 1. We make this requirement so as to - * avoid reading av_len() twice, which is hazardous for tied arrays. + * input array, i.e., av_count_limit(av). We make this requirement so as= to + * avoid reading av_count() twice, which is hazardous for tied arrays. */ static void array_to_datum_internal(AV *av, ArrayBuildState **astatep, @@ -1214,11 +1215,11 @@ array_to_datum_internal(AV *av, ArrayBuildState **= astatep, errmsg("number of array dimensions exceeds the maximum allowed (%= d)", MAXDIM))); /* OK, add a dimension */ - dims[*ndims] =3D av_len(nav) + 1; + dims[*ndims] =3D av_count_limit(nav); (*ndims)++; } else if (cur_depth >=3D *ndims || - av_len(nav) + 1 !=3D dims[cur_depth]) + av_count_limit(nav) !=3D dims[cur_depth]) ereport(ERROR, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg("multidimensional arrays must have array expressions with m= atching dimensions"))); @@ -1260,6 +1261,25 @@ array_to_datum_internal(AV *av, ArrayBuildState **a= statep, } } = +/* + * av_count returns Size_t, so at least in theory it could overrun INT_MA= X. + * As long as we have to check, let's throw error for anything above + * MaxArraySize, which will surely fail later. + */ +static int +av_count_limit(AV *av) +{ + dTHX; + Size_t cnt =3D av_count(av); + + if (cnt > MaxArraySize) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("array size exceeds the maximum allowed (%zu)", + MaxArraySize))); + return (int) cnt; +} + /* * convert perl array ref to a datum */ @@ -1287,7 +1307,7 @@ plperl_array_to_datum(SV *src, Oid typid, int32 typm= od) _sv_to_datum_finfo(elemtypid, &finfo, &typioparam); = memset(dims, 0, sizeof(dims)); - dims[0] =3D av_len(nav) + 1; + dims[0] =3D av_count_limit(nav); = array_to_datum_internal(nav, &astate, &ndims, dims, 1, @@ -2478,14 +2498,15 @@ plperl_func_handler(PG_FUNCTION_ARGS) if (sav) { dTHX; - int i =3D 0; - SV **svp =3D 0; AV *rav =3D (AV *) SvRV(sav); + Size_t alen =3D av_count(rav); = - while ((svp =3D av_fetch(rav, i, FALSE)) !=3D NULL) + for (Size_t i =3D 0; i < alen; i++) { - plperl_return_next_internal(*svp); - i++; + SV **svp =3D av_fetch(rav, i, FALSE); + + if (svp) + plperl_return_next_internal(*svp); } } else if (SvOK(perlret)) -- = 2.52.0 ------- =_aaaaaaaaaa0--