postgres.git / summary / log / commit / refs
commit c7f4628382071ef87f5e7fee8939f09db1e5e022
Author: Tom Lane <tgl@sss.pgh.pa.us>
Commit: Noah Misch <noah@leadboat.com>
Date: Mon Aug 10 13:38:35 2026 +0000
Protect some fixed-size arrays that have FUNC_MAX_ARGS elements.
The maximum number of arguments allowed for an aggregate function
is FUNC_MAX_ARGS-1 (since the underlying transfn and/or finalfn
will be called with one more argument). parse_func.c failed to
enforce this, allowing construction of calls that would try to
pass FUNC_MAX_ARGS+1 to the underlying functions, resulting in
a memory stomp in the executor. Add correct checking there.
Since it's possible that a bad call has been stored in a view or
SQL function, also add checks in various aggregate-related and
window-function-related code that there are not more than
FUNC_MAX_ARGS arguments. These will also protect us against the
possibility that we're trying to run a stored view that was made
by a server executable with different FUNC_MAX_ARGS. (Arguably,
that scenario does not qualify as a security problem. But let's
just tighten up all of this while we're here, rather than split
hairs over whether an overrun is reachable.)
Likewise check in compute_function_hashkey. Here the hazard is
directly from a pg_proc row, but the scenario is the same.
PL/Tcl has a similar issue with a fixed-size string buffer.
Let's just replace that buffer with a Tcl_DString, removing the
whole issue and making the code look more like what's around it.
There are a lot of other FUNC_MAX_ARGS-sized arrays, but the rest
have nearby guards already, some with comments explicitly pointing
out the hazard of FUNC_MAX_ARGS changing.
I also used palloc_array() in a few related places in funcapi.c.
Those aren't live hazards AFAICS, but nearby code has been
palloc_array-ified already, so it seemed inconsistent to not use
it here.
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Backpatch-through: 14
Security: CVE-2026-14679
src/backend/executor/nodeWindowAgg.c | 33 +++++++++++++++++++++++++++++++++
src/backend/parser/parse_agg.c | 18 +++++++++++++++++-
src/backend/parser/parse_func.c | 29 +++++++++++++++++++++++++++++
src/backend/utils/fmgr/funcapi.c | 6 +++---
src/pl/plpgsql/src/pl_comp.c | 14 ++++++++++++++
src/pl/tcl/pltcl.c | 22 +++++++++++++---------
6 files changed, 109 insertions(+), 13 deletions(-)
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 7933d93ba64..3d8932856d2 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -1039,6 +1039,20 @@ eval_windowfunction(WindowAggState *winstate, WindowStatePerFunc perfuncstate,
oldContext = MemoryContextSwitchTo(winstate->ss.ps.ps_ExprContext->ecxt_per_tuple_memory);
+ /*
+ * Protect fixed-size fcinfo. Ordinarily this would have been checked
+ * while creating the WindowFunc, but it's possible that we are looking at
+ * a parsetree from a stored view that was made by a server executable
+ * with a different value of FUNC_MAX_ARGS.
+ */
+ if (perfuncstate->numArguments > FUNC_MAX_ARGS)
+ ereport(ERROR,
+ (errcode(ERRCODE_TOO_MANY_ARGUMENTS),
+ errmsg_plural("cannot pass more than %d argument to a function",
+ "cannot pass more than %d arguments to a function",
+ FUNC_MAX_ARGS,
+ FUNC_MAX_ARGS)));
+
/*
* We don't pass any normal arguments to a window function, but we do pass
* it the number of arguments, in order to permit window function
@@ -2688,6 +2702,25 @@ initialize_peragg(WindowAggState *winstate, WindowFunc *wfunc,
numArguments = list_length(wfunc->args);
+ /*
+ * Check the number of arguments, to protect fixed-size arrays here and
+ * later in node execution.
+ *
+ * Aggregates can have at most FUNC_MAX_ARGS-1 args (compare
+ * AggregateCreate, whose error message we want to match). Ordinarily
+ * this would have been checked while creating the WindowFunc, but it's
+ * possible that we are looking at a parsetree from a stored view that was
+ * made by a server executable with a different value of FUNC_MAX_ARGS, or
+ * an executable in which parse_func.c didn't enforce the correct limit.
+ */
+ if (numArguments > FUNC_MAX_ARGS - 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_TOO_MANY_ARGUMENTS),
+ errmsg_plural("aggregates cannot have more than %d argument",
+ "aggregates cannot have more than %d arguments",
+ FUNC_MAX_ARGS - 1,
+ FUNC_MAX_ARGS - 1)));
+
i = 0;
foreach(lc, wfunc->args)
{
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 7a85651befe..79e6a1189a4 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -1943,7 +1943,23 @@ get_aggregate_argtypes(Aggref *aggref, Oid *inputTypes)
int numArguments = 0;
ListCell *lc;
- Assert(list_length(aggref->aggargtypes) <= FUNC_MAX_ARGS);
+ /*
+ * Check the number of arguments to protect fixed-size arrays in callers.
+ *
+ * Aggregates can have at most FUNC_MAX_ARGS-1 args (compare
+ * AggregateCreate, whose error message we want to match). Ordinarily
+ * this would have been checked while creating the Aggref, but it's
+ * possible that we are looking at a parsetree from a stored view that was
+ * made by a server executable with a different value of FUNC_MAX_ARGS, or
+ * an executable in which parse_func.c didn't enforce the correct limit.
+ */
+ if (list_length(aggref->aggargtypes) > FUNC_MAX_ARGS - 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_TOO_MANY_ARGUMENTS),
+ errmsg_plural("aggregates cannot have more than %d argument",
+ "aggregates cannot have more than %d arguments",
+ FUNC_MAX_ARGS - 1,
+ FUNC_MAX_ARGS - 1)));
foreach(lc, aggref->aggargtypes)
{
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 542f9167aa1..a644a6f5e1e 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -780,6 +780,22 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
aggref->aggtransno = -1;
aggref->location = location;
+ /*
+ * The argument-count limit for aggregates is one less than for other
+ * kinds of functions (cf. AggregateCreate). Now that we know it's an
+ * aggregate, apply the stricter limit. We need an explicit check
+ * because hypothetical-set aggregates don't have a fixed number of
+ * arguments, so having matched the pg_proc entry proves nothing.
+ */
+ if (list_length(fargs) > FUNC_MAX_ARGS - 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_TOO_MANY_ARGUMENTS),
+ errmsg_plural("aggregates cannot have more than %d argument",
+ "aggregates cannot have more than %d arguments",
+ FUNC_MAX_ARGS - 1,
+ FUNC_MAX_ARGS - 1),
+ parser_errposition(pstate, location)));
+
/*
* Reject attempt to call a parameterless aggregate without (*)
* syntax. This is mere pedantry but some folks insisted ...
@@ -844,6 +860,19 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
errmsg("DISTINCT is not implemented for window functions"),
parser_errposition(pstate, location)));
+ /*
+ * As above, enforce the correct argument-count limit if it's really
+ * an aggregate.
+ */
+ if (wfunc->winagg && list_length(fargs) > FUNC_MAX_ARGS - 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_TOO_MANY_ARGUMENTS),
+ errmsg_plural("aggregates cannot have more than %d argument",
+ "aggregates cannot have more than %d arguments",
+ FUNC_MAX_ARGS - 1,
+ FUNC_MAX_ARGS - 1),
+ parser_errposition(pstate, location)));
+
/*
* Reject attempt to call a parameterless aggregate without (*)
* syntax. This is mere pedantry but some folks insisted ...
diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c
index d7e0efda329..edfaece8818 100644
--- a/src/backend/utils/fmgr/funcapi.c
+++ b/src/backend/utils/fmgr/funcapi.c
@@ -1341,7 +1341,7 @@ get_func_arg_info(HeapTuple procTup,
ARR_ELEMTYPE(arr) != OIDOID)
elog(ERROR, "proallargtypes is not a 1-D Oid array or it contains nulls");
Assert(numargs >= procStruct->pronargs);
- *p_argtypes = (Oid *) palloc(numargs * sizeof(Oid));
+ *p_argtypes = palloc_array(Oid, numargs);
memcpy(*p_argtypes, ARR_DATA_PTR(arr),
numargs * sizeof(Oid));
}
@@ -1350,7 +1350,7 @@ get_func_arg_info(HeapTuple procTup,
/* If no proallargtypes, use proargtypes */
numargs = procStruct->proargtypes.dim1;
Assert(numargs == procStruct->pronargs);
- *p_argtypes = (Oid *) palloc(numargs * sizeof(Oid));
+ *p_argtypes = palloc_array(Oid, numargs);
memcpy(*p_argtypes, procStruct->proargtypes.values,
numargs * sizeof(Oid));
}
@@ -1430,7 +1430,7 @@ get_func_trftypes(HeapTuple procTup,
ARR_HASNULL(arr) ||
ARR_ELEMTYPE(arr) != OIDOID)
elog(ERROR, "protrftypes is not a 1-D Oid array or it contains nulls");
- *p_trftypes = (Oid *) palloc(nelems * sizeof(Oid));
+ *p_trftypes = palloc_array(Oid, nelems);
memcpy(*p_trftypes, ARR_DATA_PTR(arr),
nelems * sizeof(Oid));
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index fdaba544b1b..84a119a08e9 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -2488,6 +2488,20 @@ compute_function_hashkey(FunctionCallInfo fcinfo,
if (procStruct->pronargs > 0)
{
+ /*
+ * Protect against overrun of fixed-size hashkey->argtypes array.
+ * Ordinarily the parser would have checked this long since, but it's
+ * possible that we are looking at a pg_proc entry that was made by a
+ * server executable with a different value of FUNC_MAX_ARGS.
+ */
+ if (procStruct->pronargs > FUNC_MAX_ARGS)
+ ereport(ERROR,
+ (errcode(ERRCODE_TOO_MANY_ARGUMENTS),
+ errmsg_plural("cannot pass more than %d argument to a function",
+ "cannot pass more than %d arguments to a function",
+ FUNC_MAX_ARGS,
+ FUNC_MAX_ARGS)));
+
/* get the argument types */
memcpy(hashkey->argtypes, procStruct->proargtypes.values,
procStruct->pronargs * sizeof(Oid));
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index c4f9c7f840f..3f2bf00c6ec 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -1405,6 +1405,7 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
volatile MemoryContext proc_cxt = NULL;
Tcl_DString proc_internal_def;
Tcl_DString proc_internal_body;
+ Tcl_DString proc_internal_args;
/* We'll need the pg_proc tuple in any case... */
procTup = SearchSysCache1(PROCOID, ObjectIdGetDatum(fn_oid));
@@ -1452,17 +1453,17 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
************************************************************/
Tcl_DStringInit(&proc_internal_def);
Tcl_DStringInit(&proc_internal_body);
+ Tcl_DStringInit(&proc_internal_args);
PG_TRY();
{
bool is_trigger = OidIsValid(tgreloid);
char internal_proname[128];
HeapTuple typeTup;
Form_pg_type typeStruct;
- char proc_internal_args[33 * FUNC_MAX_ARGS];
Datum prosrcdatum;
bool isnull;
char *proc_source;
- char buf[48];
+ char buf[64];
Tcl_Interp *interp;
int i;
int tcl_rc;
@@ -1572,7 +1573,6 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
************************************************************/
if (!is_trigger && !is_event_trigger)
{
- proc_internal_args[0] = '\0';
for (i = 0; i < prodesc->nargs; i++)
{
Oid argtype = procStruct->proargtypes.values[i];
@@ -1605,8 +1605,8 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
}
if (i > 0)
- strcat(proc_internal_args, " ");
- strcat(proc_internal_args, buf);
+ Tcl_DStringAppend(&proc_internal_args, " ", -1);
+ Tcl_DStringAppend(&proc_internal_args, buf, -1);
ReleaseSysCache(typeTup);
}
@@ -1614,13 +1614,14 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
else if (is_trigger)
{
/* trigger procedure has fixed args */
- strcpy(proc_internal_args,
- "TG_name TG_relid TG_table_name TG_table_schema TG_relatts TG_when TG_level TG_op __PLTcl_Tup_NEW __PLTcl_Tup_OLD args");
+ Tcl_DStringAppend(&proc_internal_args,
+ "TG_name TG_relid TG_table_name TG_table_schema TG_relatts TG_when TG_level TG_op __PLTcl_Tup_NEW __PLTcl_Tup_OLD args",
+ -1);
}
else if (is_event_trigger)
{
/* event trigger procedure has fixed args */
- strcpy(proc_internal_args, "TG_event TG_tag");
+ Tcl_DStringAppend(&proc_internal_args, "TG_event TG_tag", -1);
}
/************************************************************
@@ -1633,7 +1634,8 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
************************************************************/
Tcl_DStringAppendElement(&proc_internal_def, "proc");
Tcl_DStringAppendElement(&proc_internal_def, internal_proname);
- Tcl_DStringAppendElement(&proc_internal_def, proc_internal_args);
+ Tcl_DStringAppendElement(&proc_internal_def,
+ Tcl_DStringValue(&proc_internal_args));
/************************************************************
* prefix procedure body with
@@ -1715,6 +1717,7 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
MemoryContextDelete(proc_cxt);
Tcl_DStringFree(&proc_internal_def);
Tcl_DStringFree(&proc_internal_body);
+ Tcl_DStringFree(&proc_internal_args);
PG_RE_THROW();
}
PG_END_TRY();
@@ -1743,6 +1746,7 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
Tcl_DStringFree(&proc_internal_def);
Tcl_DStringFree(&proc_internal_body);
+ Tcl_DStringFree(&proc_internal_args);
ReleaseSysCache(procTup);
[parent: 57aa21f69127]