agora inbox for pgsql-committers@postgresql.orghelp / color / mirror / Atom feed
pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. 6+ messages / 1 participants [nested] [flat]
* pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. @ 2026-08-10 13:41 Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 6+ messages in thread From: Noah Misch @ 2026-08-10 13:41 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/13bb73aeb688632872d9866c60be8839cdc1f599 Author: Tom Lane <tgl@sss.pgh.pa.us> Modified Files -------------- 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(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. @ 2026-08-10 13:41 Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 6+ messages in thread From: Noah Misch @ 2026-08-10 13:41 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 Branch ------ REL_19_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/86cd82bf4887cd7abf2f0203d3e1a09e7022746d Author: Tom Lane <tgl@sss.pgh.pa.us> Modified Files -------------- 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(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. @ 2026-08-10 13:41 Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 6+ messages in thread From: Noah Misch @ 2026-08-10 13:41 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 Branch ------ REL_18_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/3392cce5c92ea4c84b8a5088b4717066fd280056 Author: Tom Lane <tgl@sss.pgh.pa.us> Modified Files -------------- 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(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. @ 2026-08-10 13:41 Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 6+ messages in thread From: Noah Misch @ 2026-08-10 13:41 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/c2b16f5d495d977d669c7ec5db07bb666f3691e7 Author: Tom Lane <tgl@sss.pgh.pa.us> Modified Files -------------- 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(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. @ 2026-08-10 13:41 Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 6+ messages in thread From: Noah Misch @ 2026-08-10 13:41 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/3299c2ba0cc5034d102eaf2f146e2794e00620e1 Author: Tom Lane <tgl@sss.pgh.pa.us> Modified Files -------------- 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(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. @ 2026-08-10 13:41 Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 6+ messages in thread From: Noah Misch @ 2026-08-10 13:41 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/71ba5705d685ea56fc31309f5691c887275a632a Author: Tom Lane <tgl@sss.pgh.pa.us> Modified Files -------------- 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(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-08-10 13:41 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-08-10 13:41 pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. Noah Misch <noah@leadboat.com> 2026-08-10 13:41 pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. Noah Misch <noah@leadboat.com> 2026-08-10 13:41 pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. Noah Misch <noah@leadboat.com> 2026-08-10 13:41 pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. Noah Misch <noah@leadboat.com> 2026-08-10 13:41 pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. Noah Misch <noah@leadboat.com> 2026-08-10 13:41 pgsql: pg_dump: avoid assuming how long pg_proc.protrftypes can be. Noah Misch <noah@leadboat.com>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox