diff options
author | Jenkins2 <jenkins2@gerrit.asterisk.org> | 2017-08-31 06:07:57 -0500 |
---|---|---|
committer | Gerrit Code Review <gerrit2@gerrit.digium.api> | 2017-08-31 06:07:57 -0500 |
commit | 3502aba96e0e9ae7194dabc80527b523a42e1ea6 (patch) | |
tree | 3cd253ab0b1cfc0c21625428a9a633eda555eec5 /apps | |
parent | ff23bf1e75a3630ccd9c0d7262de64edb2823a88 (diff) | |
parent | 9c20596400cef759ecc760e1b028fd1748e9be5d (diff) |
Merge "AST-2017-006: Fix app_minivm application MinivmNotify command injection" into 14
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 41fd5e77d..647746048 100644 --- a/apps/app_minivm.c +++ b/apps/app_minivm.c @@ -1776,21 +1776,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 8de4995af..7fef15ebf 100644 --- a/apps/app_mixmonitor.c +++ b/apps/app_mixmonitor.c @@ -138,6 +138,11 @@ ASTERISK_REGISTER_FILE() <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> @@ -150,6 +155,11 @@ ASTERISK_REGISTER_FILE() <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> @@ -224,6 +234,11 @@ ASTERISK_REGISTER_FILE() <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 7fcffb196..74d33119f 100644 --- a/apps/app_system.c +++ b/apps/app_system.c @@ -48,6 +48,11 @@ ASTERISK_REGISTER_FILE() <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> @@ -73,6 +78,11 @@ ASTERISK_REGISTER_FILE() <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> |