postgres.git / summary / log / commit / refs
commit 86cd82bf4887cd7abf2f0203d3e1a09e7022746d
Author: Tom Lane <tgl@sss.pgh.pa.us>
Commit: Noah Misch <noah@leadboat.com>
Date: Mon Aug 10 13:38:04 2026 +0000
pg_dump: avoid assuming how long pg_proc.protrftypes can be.
The backend doesn't impose any particular limit on the length of this
array, and since there could be entries for both input and output
arguments, it's feasible for the length to exceed FUNC_MAX_ARGS
even without funny business. This could lead to crashes or worse.
Moreover, pg_dump shouldn't rely on hard-coding FUNC_MAX_ARGS in the
first place: it has no business assuming that the backend it's dumping
from was compiled with the same value of FUNC_MAX_ARGS that it is.
So the stanza in dumpFunc() that allocates exactly FUNC_MAX_ARGS space
for the parsed OID array is fundamentally misguided. And it's broken
in another way too: if there are exactly FUNC_MAX_ARGS OIDs, then
parseOidArray won't zero-fill any entries, allowing the subsequent
loop to run off the end of the array. A crash seems unlikely in
this variant, but garbage output is certain.
To fix, redesign parseOidArray's API so that it does the
array-mallocing, which simplifies the callers anyway. While we're
here, tighten and modernize it a bit; in particular, split it into
separate functions for OIDs and integers, as was foreseen long ago.
This lets us get rid of the confusing type-punning involved in
having IndxInfo.indkeys be declared as "Oid *" when it's really
potentially-signed ints. Also, most of the callers expect an exact
number of array entries, so make it verify that not just check for
"too many".
I noted while testing that this dumpFunc() stanza isn't even reached
during check-world. Add a function with transform to the regression
tests to rectify that.
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-19385
src/bin/pg_dump/common.c | 142 ++++++++++++++++++++++-----
src/bin/pg_dump/pg_dump.c | 30 ++----
src/bin/pg_dump/pg_dump.h | 5 +-
src/test/regress/expected/object_address.out | 4 +
src/test/regress/sql/object_address.sql | 4 +
5 files changed, 140 insertions(+), 45 deletions(-)
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index dc98c5c5c09..047e1c6bb35 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -1103,50 +1103,146 @@ findOwningExtension(CatalogId catalogId)
/*
* parseOidArray
- * parse a string of numbers delimited by spaces into a character array
+ * parse a string of unsigned numbers separated by spaces
+ * into an array of OIDs
*
- * Note: actually this is used for both Oids and potentially-signed
- * attribute numbers. This should cause no trouble, but we could split
- * the function into two functions with different argument types if it does.
+ * The result is a malloc'd array.
+ *
+ * If arraysize >= 0, we insist that the input contain exactly that many
+ * OIDs, and the allocated array is of that length too. If arraysize < 0,
+ * we dynamically size the array to have one more entry than the input
+ * provides, and fill the extra entry with zero.
*/
-
-void
-parseOidArray(const char *str, Oid *array, int arraysize)
+Oid *
+parseOidArray(const char *str, int arraysize)
{
- int j,
- argNum;
- char temp[100];
- char s;
-
+ Oid *array;
+ int allocsize,
+ argNum,
+ templen;
+ char temp[32];
+
+ if (arraysize >= 0)
+ allocsize = arraysize;
+ else
+ {
+ /*
+ * Make enough room for input + one extra entry (could be more than
+ * enough, if there are redundant spaces in the input).
+ */
+ allocsize = 2;
+ for (const char *s1 = str; *s1; s1++)
+ {
+ if (*s1 == ' ')
+ allocsize++;
+ }
+ }
+ array = pg_malloc_array(Oid, allocsize);
argNum = 0;
- j = 0;
- for (;;)
+ templen = 0;
+ for (const char *s1 = str;; s1++)
{
- s = *str++;
+ char s = *s1;
+
if (s == ' ' || s == '\0')
{
- if (j > 0)
+ if (templen > 0)
{
- if (argNum >= arraysize)
+ if (arraysize >= 0 && argNum >= arraysize)
pg_fatal("could not parse numeric array \"%s\": too many numbers", str);
- temp[j] = '\0';
+ temp[templen] = '\0';
array[argNum++] = atooid(temp);
- j = 0;
+ templen = 0;
}
if (s == '\0')
break;
}
else
{
- if (!(isdigit((unsigned char) s) || s == '-') ||
- j >= sizeof(temp) - 1)
+ if (!isdigit((unsigned char) s) ||
+ templen >= sizeof(temp) - 1)
pg_fatal("could not parse numeric array \"%s\": invalid character in number", str);
- temp[j++] = s;
+ temp[templen++] = s;
}
}
- while (argNum < arraysize)
+ if (arraysize >= 0 && argNum != arraysize)
+ pg_fatal("could not parse numeric array \"%s\": too few numbers", str);
+
+ while (argNum < allocsize)
array[argNum++] = InvalidOid;
+
+ return array;
+}
+
+
+/*
+ * parseIntArray
+ * parse a string of possibly-signed numbers separated by spaces
+ * into an array of ints
+ *
+ * This is exactly like parseOidArray, but for integers.
+ */
+int *
+parseIntArray(const char *str, int arraysize)
+{
+ int *array;
+ int allocsize,
+ argNum,
+ templen;
+ char temp[32];
+
+ if (arraysize >= 0)
+ allocsize = arraysize;
+ else
+ {
+ /*
+ * Make enough room for input + one extra entry (could be more than
+ * enough, if there are redundant spaces in the input).
+ */
+ allocsize = 2;
+ for (const char *s1 = str; *s1; s1++)
+ {
+ if (*s1 == ' ')
+ allocsize++;
+ }
+ }
+ array = pg_malloc_array(int, allocsize);
+ argNum = 0;
+ templen = 0;
+ for (const char *s1 = str;; s1++)
+ {
+ char s = *s1;
+
+ if (s == ' ' || s == '\0')
+ {
+ if (templen > 0)
+ {
+ if (arraysize >= 0 && argNum >= arraysize)
+ pg_fatal("could not parse numeric array \"%s\": too many numbers", str);
+ temp[templen] = '\0';
+ array[argNum++] = atoi(temp);
+ templen = 0;
+ }
+ if (s == '\0')
+ break;
+ }
+ else
+ {
+ if (!(isdigit((unsigned char) s) || s == '-') ||
+ templen >= sizeof(temp) - 1)
+ pg_fatal("could not parse numeric array \"%s\": invalid character in number", str);
+ temp[templen++] = s;
+ }
+ }
+
+ if (arraysize >= 0 && argNum != arraysize)
+ pg_fatal("could not parse numeric array \"%s\": too few numbers", str);
+
+ while (argNum < allocsize)
+ array[argNum++] = 0;
+
+ return array;
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 41b9531e41a..0abc80ea5b6 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -6976,12 +6976,8 @@ getAggregates(Archive *fout)
if (agginfo[i].aggfn.nargs == 0)
agginfo[i].aggfn.argtypes = NULL;
else
- {
- agginfo[i].aggfn.argtypes = pg_malloc_array(Oid, agginfo[i].aggfn.nargs);
- parseOidArray(PQgetvalue(res, i, i_proargtypes),
- agginfo[i].aggfn.argtypes,
- agginfo[i].aggfn.nargs);
- }
+ agginfo[i].aggfn.argtypes = parseOidArray(PQgetvalue(res, i, i_proargtypes),
+ agginfo[i].aggfn.nargs);
agginfo[i].aggfn.postponed_def = false; /* might get set during sort */
/* Decide whether we want to dump it */
@@ -7169,11 +7165,8 @@ getFuncs(Archive *fout)
if (finfo[i].nargs == 0)
finfo[i].argtypes = NULL;
else
- {
- finfo[i].argtypes = pg_malloc_array(Oid, finfo[i].nargs);
- parseOidArray(PQgetvalue(res, i, i_proargtypes),
- finfo[i].argtypes, finfo[i].nargs);
- }
+ finfo[i].argtypes = parseOidArray(PQgetvalue(res, i, i_proargtypes),
+ finfo[i].nargs);
finfo[i].postponed_def = false; /* might get set during sort */
/* Decide whether we want to dump it */
@@ -8247,9 +8240,8 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
indxinfo[j].indreloptions = pg_strdup(PQgetvalue(res, j, i_indreloptions));
indxinfo[j].indstatcols = pg_strdup(PQgetvalue(res, j, i_indstatcols));
indxinfo[j].indstatvals = pg_strdup(PQgetvalue(res, j, i_indstatvals));
- indxinfo[j].indkeys = pg_malloc_array(Oid, indxinfo[j].indnattrs);
- parseOidArray(PQgetvalue(res, j, i_indkey),
- indxinfo[j].indkeys, indxinfo[j].indnattrs);
+ indxinfo[j].indkeys = parseIntArray(PQgetvalue(res, j, i_indkey),
+ indxinfo[j].indnattrs);
indxinfo[j].indisclustered = (PQgetvalue(res, j, i_indisclustered)[0] == 't');
indxinfo[j].indisreplident = (PQgetvalue(res, j, i_indisreplident)[0] == 't');
indxinfo[j].indnullsnotdistinct = (PQgetvalue(res, j, i_indnullsnotdistinct)[0] == 't');
@@ -13868,12 +13860,10 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
if (*protrftypes)
{
- Oid *typeids = pg_malloc_array(Oid, FUNC_MAX_ARGS);
- int i;
+ Oid *typeids = parseOidArray(protrftypes, -1);
appendPQExpBufferStr(q, " TRANSFORM ");
- parseOidArray(protrftypes, typeids, FUNC_MAX_ARGS);
- for (i = 0; typeids[i]; i++)
+ for (int i = 0; typeids[i]; i++)
{
if (i != 0)
appendPQExpBufferStr(q, ", ");
@@ -19123,7 +19113,7 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo)
appendPQExpBufferStr(q, " (");
for (k = 0; k < indxinfo->indnkeyattrs; k++)
{
- int indkey = (int) indxinfo->indkeys[k];
+ int indkey = indxinfo->indkeys[k];
const char *attname;
if (indkey == InvalidAttrNumber)
@@ -19142,7 +19132,7 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo)
for (k = indxinfo->indnkeyattrs; k < indxinfo->indnattrs; k++)
{
- int indkey = (int) indxinfo->indkeys[k];
+ int indkey = indxinfo->indkeys[k];
const char *attname;
if (indkey == InvalidAttrNumber)
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 5a6726d8b12..e6eefa98460 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -426,7 +426,7 @@ typedef struct _indxInfo
char *indstatvals; /* statistic values for columns */
int indnkeyattrs; /* number of index key attributes */
int indnattrs; /* total number of index attributes */
- Oid *indkeys; /* In spite of the name 'indkeys' this field
+ int *indkeys; /* In spite of the name 'indkeys' this field
* contains both key and nonkey attributes */
bool indisclustered;
bool indisreplident;
@@ -782,7 +782,8 @@ extern SubscriptionInfo *findSubscriptionByOid(Oid oid);
extern void recordExtensionMembership(CatalogId catId, ExtensionInfo *ext);
extern ExtensionInfo *findOwningExtension(CatalogId catalogId);
-extern void parseOidArray(const char *str, Oid *array, int arraysize);
+extern Oid *parseOidArray(const char *str, int arraysize);
+extern int *parseIntArray(const char *str, int arraysize);
extern void sortDumpableObjects(DumpableObject **objs, int numObjs,
DumpId preBoundaryId, DumpId postBoundaryId);
diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out
index f776865ce14..cf36cb7f4f3 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -44,6 +44,10 @@ ALTER DEFAULT PRIVILEGES FOR ROLE regress_addr_user REVOKE DELETE ON TABLES FROM
CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION prsd_lextype(internal),
TO SQL WITH FUNCTION int4recv(internal));
+-- make a function that uses it too, mainly to exercise pg_dump
+CREATE FUNCTION public.sql_func_with_transform(int) RETURNS int LANGUAGE sql
+AS 'select $1 + 1'
+TRANSFORM FOR TYPE int;
-- suppress warning that depends on wal_level
SET client_min_messages = 'ERROR';
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
diff --git a/src/test/regress/sql/object_address.sql b/src/test/regress/sql/object_address.sql
index ed795a32e22..b274b42e137 100644
--- a/src/test/regress/sql/object_address.sql
+++ b/src/test/regress/sql/object_address.sql
@@ -47,6 +47,10 @@ ALTER DEFAULT PRIVILEGES FOR ROLE regress_addr_user REVOKE DELETE ON TABLES FROM
CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION prsd_lextype(internal),
TO SQL WITH FUNCTION int4recv(internal));
+-- make a function that uses it too, mainly to exercise pg_dump
+CREATE FUNCTION public.sql_func_with_transform(int) RETURNS int LANGUAGE sql
+AS 'select $1 + 1'
+TRANSFORM FOR TYPE int;
-- suppress warning that depends on wal_level
SET client_min_messages = 'ERROR';
CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable;
[parent: 92972e815d85]