diff options
author | Joshua Colp <jcolp@digium.com> | 2017-08-31 07:57:15 -0500 |
---|---|---|
committer | Gerrit Code Review <gerrit2@gerrit.digium.api> | 2017-08-31 07:57:15 -0500 |
commit | d6f5e475ce5149b5b8020502250035ff3fffe02d (patch) | |
tree | dff9f48306a20869ddc86f05e0a795163deba227 /apps | |
parent | b006220ebc101eb4acfd917519b75b04681e97ad (diff) | |
parent | 0372157a4875bb8e76da9aecbe84a64307ffafe7 (diff) |
Merge "AST-2017-006: Fix app_minivm application MinivmNotify command injection" into 15
Diffstat (limited to 'apps')
-rw-r--r-- | apps/app_minivm.c | 36 | ||||
-rw-r--r-- | apps/app_mixmonitor.c | 15 | ||||
-rw-r--r-- | apps/app_system.c | 10 |
3 files changed, 50 insertions, 11 deletions
diff --git a/apps/app_minivm.c b/apps/app_minivm.c index 15449ad4e..0d7a5f407 100644 --- a/apps/app_minivm.c +++ b/apps/app_minivm.c @@ -1774,21 +1774,35 @@ static int play_record_review(struct ast_channel *chan, char *playfile, char *re /*! \brief Run external notification for voicemail message */ static void run_externnotify(struct ast_channel *chan, struct minivm_account *vmu) { - char arguments[BUFSIZ]; + char fquser[AST_MAX_CONTEXT * 2]; + char *argv[5] = { NULL }; + struct ast_party_caller *caller; + char *cid; + int idx; - if (ast_strlen_zero(vmu->externnotify) && ast_strlen_zero(global_externnotify)) + if (ast_strlen_zero(vmu->externnotify) && ast_strlen_zero(global_externnotify)) { return; + } + + snprintf(fquser, sizeof(fquser), "%s@%s", vmu->username, vmu->domain); - snprintf(arguments, sizeof(arguments), "%s %s@%s %s %s&", - ast_strlen_zero(vmu->externnotify) ? global_externnotify : vmu->externnotify, - vmu->username, vmu->domain, - (ast_channel_caller(chan)->id.name.valid && ast_channel_caller(chan)->id.name.str) - ? ast_channel_caller(chan)->id.name.str : "", - (ast_channel_caller(chan)->id.number.valid && ast_channel_caller(chan)->id.number.str) - ? ast_channel_caller(chan)->id.number.str : ""); + caller = ast_channel_caller(chan); + idx = 0; + argv[idx++] = ast_strlen_zero(vmu->externnotify) ? global_externnotify : vmu->externnotify; + argv[idx++] = fquser; + cid = S_COR(caller->id.name.valid, caller->id.name.str, NULL); + if (cid) { + argv[idx++] = cid; + } + cid = S_COR(caller->id.number.valid, caller->id.number.str, NULL); + if (cid) { + argv[idx++] = cid; + } + argv[idx] = NULL; - ast_debug(1, "Executing: %s\n", arguments); - ast_safe_system(arguments); + ast_debug(1, "Executing: %s %s %s %s\n", + argv[0], argv[1], argv[2] ?: "", argv[3] ?: ""); + ast_safe_execvp(1, argv[0], argv); } /*!\internal diff --git a/apps/app_mixmonitor.c b/apps/app_mixmonitor.c index 3258b301f..ac4564205 100644 --- a/apps/app_mixmonitor.c +++ b/apps/app_mixmonitor.c @@ -136,6 +136,11 @@ <para>Will be executed when the recording is over.</para> <para>Any strings matching <literal>^{X}</literal> will be unescaped to <variable>X</variable>.</para> <para>All variables will be evaluated at the time MixMonitor is called.</para> + <warning><para>Do not use untrusted strings such as <variable>CALLERID(num)</variable> + or <variable>CALLERID(name)</variable> as part of the command parameters. You + risk a command injection attack executing arbitrary commands if the untrusted + strings aren't filtered to remove dangerous characters. See function + <variable>FILTER()</variable>.</para></warning> </parameter> </syntax> <description> @@ -148,6 +153,11 @@ <para>Will contain the filename used to record.</para> </variable> </variablelist> + <warning><para>Do not use untrusted strings such as <variable>CALLERID(num)</variable> + or <variable>CALLERID(name)</variable> as part of ANY of the application's + parameters. You risk a command injection attack executing arbitrary commands + if the untrusted strings aren't filtered to remove dangerous characters. See + function <variable>FILTER()</variable>.</para></warning> </description> <see-also> <ref type="application">Monitor</ref> @@ -222,6 +232,11 @@ <para>Will be executed when the recording is over. Any strings matching <literal>^{X}</literal> will be unescaped to <variable>X</variable>. All variables will be evaluated at the time MixMonitor is called.</para> + <warning><para>Do not use untrusted strings such as <variable>CALLERID(num)</variable> + or <variable>CALLERID(name)</variable> as part of the command parameters. You + risk a command injection attack executing arbitrary commands if the untrusted + strings aren't filtered to remove dangerous characters. See function + <variable>FILTER()</variable>.</para></warning> </parameter> </syntax> <description> diff --git a/apps/app_system.c b/apps/app_system.c index 09179f7f7..64d529798 100644 --- a/apps/app_system.c +++ b/apps/app_system.c @@ -46,6 +46,11 @@ <syntax> <parameter name="command" required="true"> <para>Command to execute</para> + <warning><para>Do not use untrusted strings such as <variable>CALLERID(num)</variable> + or <variable>CALLERID(name)</variable> as part of the command parameters. You + risk a command injection attack executing arbitrary commands if the untrusted + strings aren't filtered to remove dangerous characters. See function + <variable>FILTER()</variable>.</para></warning> </parameter> </syntax> <description> @@ -71,6 +76,11 @@ <syntax> <parameter name="command" required="true"> <para>Command to execute</para> + <warning><para>Do not use untrusted strings such as <variable>CALLERID(num)</variable> + or <variable>CALLERID(name)</variable> as part of the command parameters. You + risk a command injection attack executing arbitrary commands if the untrusted + strings aren't filtered to remove dangerous characters. See function + <variable>FILTER()</variable>.</para></warning> </parameter> </syntax> <description> |