Assert fcinfo has enough args before allowing parameter access (was: Re: generate_series for timestamptz and time zone problem)
On Mon, Jan 30, 2023 at 11:50 PM Gurjeet Singh <gurjeet@singh.im> wrote:
It was the classical case of out-of-bounds access.
This mistake would've been caught early if there were assertions
preventing access beyond the number of arguments passed to the
function. I'll send the assert_enough_args.patch, that adds these
checks, in a separate thread to avoid potentially confusing cfbot.
Please see attached the patch to that ensures we don't accidentally
access more parameters than that are passed to a SQL callable
function.
Best regards,
Gurjeet
http://Gurje.et
Attachments:
assert_enough_args.patchapplication/x-patch; name=assert_enough_args.patchDownload
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index b120f5e7fe..a445ac56b9 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -206,7 +206,7 @@ extern void fmgr_symbol(Oid functionId, char **mod, char **fn);
* If function is not marked "proisstrict" in pg_proc, it must check for
* null arguments using this macro. Do not try to GETARG a null argument!
*/
-#define PG_ARGISNULL(n) (fcinfo->args[n].isnull)
+#define PG_ARGISNULL(n) (AssertMacro(n < PG_NARGS()), fcinfo->args[n].isnull)
/*
* Support for fetching detoasted copies of toastable datatypes (all of
@@ -265,7 +265,7 @@ extern struct varlena *pg_detoast_datum_packed(struct varlena *datum);
/* Macros for fetching arguments of standard types */
-#define PG_GETARG_DATUM(n) (fcinfo->args[n].value)
+#define PG_GETARG_DATUM(n) (AssertMacro(n < PG_NARGS()), fcinfo->args[n].value)
#define PG_GETARG_INT32(n) DatumGetInt32(PG_GETARG_DATUM(n))
#define PG_GETARG_UINT32(n) DatumGetUInt32(PG_GETARG_DATUM(n))
#define PG_GETARG_INT16(n) DatumGetInt16(PG_GETARG_DATUM(n))
Gurjeet Singh <gurjeet@singh.im> writes:
Please see attached the patch to that ensures we don't accidentally
access more parameters than that are passed to a SQL callable
function.
I'm unexcited by that. It'd add a pretty substantial amount
of code to catch an error that hardly anyone ever makes.
regards, tom lane