From d847f7e8f8736e5f51fd11df2992fac87131c1d8 Mon Sep 17 00:00:00 2001 From: Jaco Kroon Date: Wed, 17 Feb 2021 22:51:17 +0200 Subject: [PATCH] func_odbc: Introduce minargs config and expose ARGC in addition to ARGn. minargs enables enforcing of minimum count of arguments to pass to func_odbc, so if you're unconditionally using ARG1 through ARG4 then this should be set to 4. func_odbc will generate an error in this case, so for example [FOO] minargs = 4 and ODBC_FOO(a,b,c) in dialplan will now error out instead of using a potentially leaked ARG4 from Gosub(). ARGC is needed if you're using optional argument, to verify whether or not an argument has been passed, else it's possible to use a leaked ARGn from Gosub (app_stack). So now you can safely do ${IF($[${ARGC}>3]?${ARGV}:default value)} kind of thing. Change-Id: I6ca0b137d90b03f6aa9c496991f6cbf1518f6c24 Signed-off-by: Jaco Kroon --- configs/samples/func_odbc.conf.sample | 11 +++++++ .../func_odbc_ARGC_minargs.txt | 20 ++++++++++++ funcs/func_odbc.c | 31 +++++++++++++++++-- 3 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 doc/CHANGES-staging/func_odbc_ARGC_minargs.txt diff --git a/configs/samples/func_odbc.conf.sample b/configs/samples/func_odbc.conf.sample index c467f7ec06..b825974ea7 100644 --- a/configs/samples/func_odbc.conf.sample +++ b/configs/samples/func_odbc.conf.sample @@ -23,6 +23,10 @@ ; For substitution, you have ${ARG1}, ${ARG2} ... ${ARGn} ; for the arguments to each SQL statement. ; +; Additionally you can use ${ARGC} to determine the number of arguments that +; was actually passed (or risk using leaked ARGn variables from the channel). +; Also reference the minargs configuration option. +; ; In addition, for write statements, you have ${VAL1}, ${VAL2} ... ${VALn} ; parsed, just like arguments, for the values. In addition, if you want the ; whole value, never mind the parsing, you can get that with ${VALUE}. @@ -87,6 +91,13 @@ ; These additional rows can be returned by using the name of the ; function which was called to retrieve the first row as an ; argument to ODBC_FETCH(). +; minargs The minimum number of ARGUMENTS that has to be passed to the +; function. If fewer arguments than this is passed, then the call +; will fail. It is important to note that unlike Gosub() and friends, +; func_odbc will not mask out ARGn variables that it's not actively +; using, as such, without this, it's entirely possible to use say +; ARG2 from the Gosub() inside func_odbc when the intent was to +; use an argument passed to func_odbc, but it simply was never passed. ; ODBC_SQL - Allow an SQL statement to be built entirely in the dialplan diff --git a/doc/CHANGES-staging/func_odbc_ARGC_minargs.txt b/doc/CHANGES-staging/func_odbc_ARGC_minargs.txt new file mode 100644 index 0000000000..0984b5022d --- /dev/null +++ b/doc/CHANGES-staging/func_odbc_ARGC_minargs.txt @@ -0,0 +1,20 @@ +Subject: func_odbc + +Introduce an ARGC variable for func_odbc functions, along with a minargs +per-function configuration option. + +minargs enables enforcing of minimum count of arguments to pass to +func_odbc, so if you're unconditionally using ARG1 through ARG4 then +this should be set to 4. func_odbc will generate an error in this case, +so for example + +[FOO] +minargs = 4 + +and ODBC_FOO(a,b,c) in dialplan will now error out instead of using a +potentially leaked ARG4 from Gosub(). + +ARGC is needed if you're using optional argument, to verify whether or +not an argument has been passed, else it's possible to use a leaked ARGn +from Gosub (app_stack). So now you can safely do +${IF($[${ARGC}>3]?${ARGV}:default value)} kind of thing. diff --git a/funcs/func_odbc.c b/funcs/func_odbc.c index 5cc0faaa98..9d6d0fc304 100644 --- a/funcs/func_odbc.c +++ b/funcs/func_odbc.c @@ -120,6 +120,7 @@ struct acf_odbc_query { char *sql_insert; unsigned int flags; int rowlimit; + int minargs; struct ast_custom_function *acf; }; @@ -545,6 +546,14 @@ static int acf_odbc_write(struct ast_channel *chan, const char *cmd, char *s, co return -1; } + AST_STANDARD_APP_ARGS(args, s); + if (args.argc < query->minargs) { + ast_log(LOG_ERROR, "%d arguments supplied to '%s' requiring minimum %d\n", + args.argc, cmd, query->minargs); + AST_RWLIST_UNLOCK(&queries); + return -1; + } + if (!chan) { if (!(chan = ast_dummy_channel_alloc())) { AST_RWLIST_UNLOCK(&queries); @@ -578,7 +587,8 @@ static int acf_odbc_write(struct ast_channel *chan, const char *cmd, char *s, co return -1; } - AST_STANDARD_APP_ARGS(args, s); + snprintf(varname, sizeof(varname), "%u", args.argc); + pbx_builtin_pushvar_helper(chan, "ARGC", varname); for (i = 0; i < args.argc; i++) { snprintf(varname, sizeof(varname), "ARG%d", i + 1); pbx_builtin_pushvar_helper(chan, varname, args.field[i]); @@ -603,6 +613,8 @@ static int acf_odbc_write(struct ast_channel *chan, const char *cmd, char *s, co chan = ast_channel_unref(chan); } else { /* Restore prior values */ + pbx_builtin_setvar_helper(chan, "ARGC", NULL); + for (i = 0; i < args.argc; i++) { snprintf(varname, sizeof(varname), "ARG%d", i + 1); pbx_builtin_setvar_helper(chan, varname, NULL); @@ -756,6 +768,14 @@ static int acf_odbc_read(struct ast_channel *chan, const char *cmd, char *s, cha return -1; } + AST_STANDARD_APP_ARGS(args, s); + if (args.argc < query->minargs) { + ast_log(LOG_ERROR, "%d arguments supplied to '%s' requiring minimum %d\n", + args.argc, cmd, query->minargs); + AST_RWLIST_UNLOCK(&queries); + return -1; + } + if (!chan) { if (!(chan = ast_dummy_channel_alloc())) { AST_RWLIST_UNLOCK(&queries); @@ -768,7 +788,8 @@ static int acf_odbc_read(struct ast_channel *chan, const char *cmd, char *s, cha ast_autoservice_start(chan); } - AST_STANDARD_APP_ARGS(args, s); + snprintf(varname, sizeof(varname), "%u", args.argc); + pbx_builtin_pushvar_helper(chan, "ARGC", varname); for (x = 0; x < args.argc; x++) { snprintf(varname, sizeof(varname), "ARG%d", x + 1); pbx_builtin_pushvar_helper(chan, varname, args.field[x]); @@ -780,6 +801,8 @@ static int acf_odbc_read(struct ast_channel *chan, const char *cmd, char *s, cha chan = ast_channel_unref(chan); } else { /* Restore prior values */ + pbx_builtin_setvar_helper(chan, "ARGC", NULL); + for (x = 0; x < args.argc; x++) { snprintf(varname, sizeof(varname), "ARG%d", x + 1); pbx_builtin_setvar_helper(chan, varname, NULL); @@ -1290,6 +1313,10 @@ static int init_acf_query(struct ast_config *cfg, char *catg, struct acf_odbc_qu sscanf(tmp, "%30d", &((*query)->rowlimit)); } + if ((tmp = ast_variable_retrieve(cfg, catg, "minargs"))) { + sscanf(tmp, "%30d", &((*query)->minargs)); + } + (*query)->acf = ast_calloc(1, sizeof(struct ast_custom_function)); if (!(*query)->acf) { free_acf_query(*query); -- 2.26.3