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
From 51d0d098b82276657fcc4e8d80c87ba3cf514f98 Mon Sep 17 00:00:00 2001
From: Tom Lane <[email protected]>
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 <[email protected]>
Discussion: https://postgr.es/m/[email protected]
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/jsonb_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 = av_len(in) + 1;
- SSize_t i;
+ Size_t pcount = av_count(in);
pushJsonbValue(jsonb_state, WJB_BEGIN_ARRAY, NULL);
- for (i = 0; i < pcount; i++)
+ for (Size_t i = 0; i < pcount; i++)
{
SV **value = 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, ArrayBuildState **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 the
- * 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] = av_len(nav) + 1;
+ dims[*ndims] = av_count_limit(nav);
(*ndims)++;
}
else if (cur_depth >= *ndims ||
- av_len(nav) + 1 != dims[cur_depth])
+ av_count_limit(nav) != dims[cur_depth])
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
errmsg("multidimensional arrays must have array expressions with matching dimensions")));
@@ -1260,6 +1261,25 @@ array_to_datum_internal(AV *av, ArrayBuildState **astatep,
}
}
+/*
+ * av_count returns Size_t, so at least in theory it could overrun INT_MAX.
+ * 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 = 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 typmod)
_sv_to_datum_finfo(elemtypid, &finfo, &typioparam);
memset(dims, 0, sizeof(dims));
- dims[0] = av_len(nav) + 1;
+ dims[0] = 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 = 0;
- SV **svp = 0;
AV *rav = (AV *) SvRV(sav);
+ Size_t alen = av_count(rav);
- while ((svp = av_fetch(rav, i, FALSE)) != NULL)
+ for (Size_t i = 0; i < alen; i++)
{
- plperl_return_next_internal(*svp);
- i++;
+ SV **svp = av_fetch(rav, i, FALSE);
+
+ if (svp)
+ plperl_return_next_internal(*svp);
}
}
else if (SvOK(perlret))
--
2.52.0
