[asterisk-commits] jrose: trunk r363159 - in /trunk: ./ main/manager.c

[SVN commits to the Asterisk project]

Author: jrose
Date: Mon Apr 23 09:48:22 2012
New Revision: 363159

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=363159
Log:
AST-2012-004: Fix an error that allows AMI users to run shell commands sans authorization.

As detailed in the advisory, AMI users without write authorization for SYSTEM class AMI
actions were able to run system commands by going through other AMI commands which did
not require that authorization. Specifically, GetVar and Status allowed users to do this
by setting their variable/s options to the SHELL or EVAL functions.
Also, within 1.8, 10, and trunk there was a similar flaw with the Originate action that
allowed users with originate permission to run MixMonitor and supply a shell command
in the Data argument. That flaw is fixed in those versions of this patch.

(closes issue ASTERISK-17465)
Reported By: David Woolley
Patches:
	162_ami_readfunc_security_r2.diff uploaded by jrose (license 6182)
	18_ami_readfunc_security_r2.diff uploaded by jrose (license 6182)
	10_ami_readfunc_security_r2.diff uploaded by jrose (license 6182)
........

Merged revisions 363117 from http://svn.asterisk.org/svn/asterisk/branches/1.6.2
........

Merged revisions 363141 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 363156 from http://svn.asterisk.org/svn/asterisk/branches/10

Modified:
    trunk/   (props changed)
    trunk/main/manager.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.

Modified: trunk/main/manager.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/manager.c?view=diff&rev=363159&r1=363158&r2=363159
==============================================================================
--- trunk/main/manager.c (original)
+++ trunk/main/manager.c Mon Apr 23 09:48:22 2012
@@ -1221,6 +1221,19 @@
 	{ INT_MAX, "all" },
 	{ 0, "none" },
 };
+
+/*! \brief Checks to see if a string which can be used to evaluate functions should be rejected */
+static int function_capable_string_allowed_with_auths(const char *evaluating, int writepermlist)
+{
+	if (!(writepermlist & EVENT_FLAG_SYSTEM)
+		&& (
+			strstr(evaluating, "SHELL") ||       /* NoOp(${SHELL(rm -rf /)})  */
+			strstr(evaluating, "EVAL")           /* NoOp(${EVAL(${some_var_containing_SHELL})}) */
+		)) {
+		return 0;
+	}
+	return 1;
+}
 
 /*! \brief Convert authority code to a list of options */
 static const char *authority_to_str(int authority, struct ast_str **res)
@@ -3321,6 +3334,12 @@
 		return 0;
 	}
 
+	/* We don't want users with insufficient permissions using certain functions. */
+	if (!(function_capable_string_allowed_with_auths(varname, s->session->writeperm))) {
+		astman_send_error(s, m, "GetVar Access Forbidden: Variable");
+		return 0;
+	}
+
 	if (!ast_strlen_zero(name)) {
 		if (!(c = ast_channel_get_by_name(name))) {
 			astman_send_error(s, m, "No such channel");
@@ -3379,6 +3398,11 @@
 		snprintf(idText, sizeof(idText), "ActionID: %s\r\n", id);
 	} else {
 		idText[0] = '\0';
+	}
+
+	if (!(function_capable_string_allowed_with_auths(variables, s->session->writeperm))) {
+		astman_send_error(s, m, "Status Access Forbidden: Variables");
+		return 0;
 	}
 
 	if (all) {
@@ -4203,6 +4227,7 @@
 	}
 
 	if (!ast_strlen_zero(app)) {
+		int bad_appdata = 0;
 		/* To run the System application (or anything else that goes to
 		 * shell), you must have the additional System privilege */
 		if (!(s->session->writeperm & EVENT_FLAG_SYSTEM)
@@ -4213,10 +4238,13 @@
 				                                     TryExec(System(rm -rf /)) */
 				strcasestr(app, "agi") ||         /* AGI(/bin/rm,-rf /)
 				                                     EAGI(/bin/rm,-rf /)       */
-				strstr(appdata, "SHELL") ||       /* NoOp(${SHELL(rm -rf /)})  */
-				strstr(appdata, "EVAL")           /* NoOp(${EVAL(${some_var_containing_SHELL})}) */
+				strcasestr(app, "mixmonitor") ||  /* MixMonitor(blah,,rm -rf)  */
+				(strstr(appdata, "SHELL") && (bad_appdata = 1)) ||       /* NoOp(${SHELL(rm -rf /)})  */
+				(strstr(appdata, "EVAL") && (bad_appdata = 1))           /* NoOp(${EVAL(${some_var_containing_SHELL})}) */
 				)) {
-			astman_send_error(s, m, "Originate with certain 'Application' arguments requires the additional System privilege, which you do not have.");
+			char error_buf[64];
+			snprintf(error_buf, sizeof(error_buf), "Originate Access Forbidden: %s", bad_appdata ? "Data" : "Application");
+			astman_send_error(s, m, error_buf);
 			res = 0;
 			goto fast_orig_cleanup;
 		}