r22853: checkin pending security fixes and release notes updates for Samba 3.0.25samba-3.0.25

10 files changed, 294 insertions, 45 deletions

diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index 66d29cac89c..073d76b459f 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -32,6 +32,23 @@ Major features included in the 3.0.25 code base include:
     improved read performance with Linux servers.
   o Man pages for IdMap and VFS plug-ins.
 
+Security Fixes included in the Samba 3.0.25 release are:
+
+  o CVE-2007-2444
+	Versions: Samba 3.0.23d - 3.0.25pre2
+	Local SID/Name translation bug can result in
+	user privilege elevation
+
+  o CVE-2007-2446
+	Versions: Samba 3.0.0 - 3.0.24
+	Multiple heap overflows allow remote code execution
+
+  o CVE-2007-2447
+	Versions: Samba 3.0.0 - 3.0.24
+	Unescaped user input parameters are passed as
+	arguments to /bin/sh allowing for remote command
+	execution
+
 
 Off-line Logons and AD Site Support
 ===================================
@@ -144,6 +161,7 @@ o   Jeremy Allison <jra@samba.org>
     * Fix DFS MS-RPC enumeration reply when we have no DFS shares.
     * Fix memory corruption when enumerating accounts in the 
       LsaPrivilege database.
+    * Fixes for CVE-2007-2444, CVE-2007-2446, and CVE-2007-2447.
 
 
 o   Gerald (Jerry) Carter <jerry@samba.org>
@@ -177,6 +195,7 @@ o   Volker Lendecke <vl@samba.org>
     * Fix memory leak in smbd's claim session code.
     * BUG 4613: Fix incorrect password expiration caused by stomping on 
       the time values in the NET_USER_INFO_3 for remote users.
+    * Fixes for CVE-2007-2446.
 
 
 o   Stefan Metzmacher <metze@samba.org>
@@ -194,6 +213,7 @@ o   Simo Sorce <idra@samba.org>
     * Fix socket leak in idmap_ldap.c.
     * Fix failure in "net idmap restore".
     * Fix crash bug in idmap_ldap's get_credentials() code.
+    * Fixes for CVE-2007-2446.
 
 
 o   Alison Winters <alisonw@sgi.com>
diff --git a/source/include/smb_macros.h b/source/include/smb_macros.h
index 7c715884b01..d26433359be 100644
--- a/source/include/smb_macros.h
+++ b/source/include/smb_macros.h
@@ -295,7 +295,6 @@ copy an IP address from one buffer to another
 #if defined(PARANOID_MALLOC_CHECKER)
 
 #define PRS_ALLOC_MEM(ps, type, count) (type *)prs_alloc_mem_((ps),sizeof(type),(count))
-#define PRS_ALLOC_MEM_VOID(ps, size) prs_alloc_mem_((ps),(size),1)
 
 /* Get medieval on our ass about malloc.... */
 
@@ -334,7 +333,6 @@ copy an IP address from one buffer to another
 #else
 
 #define PRS_ALLOC_MEM(ps, type, count) (type *)prs_alloc_mem((ps),sizeof(type),(count))
-#define PRS_ALLOC_MEM_VOID(ps, size) prs_alloc_mem((ps),(size),1)
 
 /* Regular malloc code. */
 
diff --git a/source/lib/smbrun.c b/source/lib/smbrun.c
index 4400aeb4433..e81224b5af3 100644
--- a/source/lib/smbrun.c
+++ b/source/lib/smbrun.c
@@ -55,7 +55,7 @@ run a command being careful about uid/gid handling and putting the output in
 outfd (or discard it if outfd is NULL).
 ****************************************************************************/
 
-int smbrun(const char *cmd, int *outfd)
+static int smbrun_internal(const char *cmd, int *outfd, BOOL sanitize)
 {
 	pid_t pid;
 	uid_t uid = current_user.ut.uid;
@@ -173,13 +173,36 @@ int smbrun(const char *cmd, int *outfd)
 	}
 #endif
 
-	execl("/bin/sh","sh","-c",cmd,NULL);  
+	{
+		const char *newcmd = sanitize ? escape_shell_string(cmd) : cmd;
+		if (!newcmd) {
+			exit(82);
+		}
+		execl("/bin/sh","sh","-c",newcmd,NULL);  
+	}
 	
 	/* not reached */
-	exit(82);
+	exit(83);
 	return 1;
 }
 
+/****************************************************************************
+ Use only in known safe shell calls (printing).
+****************************************************************************/
+
+int smbrun_no_sanitize(const char *cmd, int *outfd)
+{
+	return smbrun_internal(cmd, outfd, False);
+}
+
+/****************************************************************************
+ By default this now sanitizes shell expansion.
+****************************************************************************/
+
+int smbrun(const char *cmd, int *outfd)
+{
+	return smbrun_internal(cmd, outfd, True);
+}
 
 /****************************************************************************
 run a command being careful about uid/gid handling and putting the output in
@@ -302,7 +325,7 @@ int smbrunsecret(const char *cmd, const char *secret)
 #endif
 
 	execl("/bin/sh", "sh", "-c", cmd, NULL);  
-	
+
 	/* not reached */
 	exit(82);
 	return 1;
diff --git a/source/lib/util_str.c b/source/lib/util_str.c
index 457232c2b21..1439ac6fcd3 100644
--- a/source/lib/util_str.c
+++ b/source/lib/util_str.c
@@ -2616,3 +2616,166 @@ size_t utf16_len_n(const void *src, size_t n)
 
 	return len;
 }
+
+/*******************************************************************
+ Add a shell escape character '\' to any character not in a known list
+ of characters. UNIX charset format.
+*******************************************************************/
+
+#define INCLUDE_LIST "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabdefghijklmnopqrstuvwxyz_/ \t.,"
+#define INSIDE_DQUOTE_LIST "$`\n\"\\"
+
+char *escape_shell_string(const char *src)
+{
+	size_t srclen = strlen(src);
+	char *ret = SMB_MALLOC((srclen * 2) + 1);
+	char *dest = ret;
+	BOOL in_s_quote = False;
+	BOOL in_d_quote = False;
+	BOOL next_escaped = False;
+
+	if (!ret) {
+		return NULL;
+	}
+
+	while (*src) {
+		size_t c_size;
+		codepoint_t c = next_codepoint(src, &c_size);
+
+		if (c == INVALID_CODEPOINT) {
+			SAFE_FREE(ret);
+			return NULL;
+		}
+
+		if (c_size > 1) {
+			memcpy(dest, src, c_size);
+			src += c_size;
+			dest += c_size;
+			next_escaped = False;
+			continue;
+		}
+
+		/*
+		 * Deal with backslash escaped state.
+		 * This only lasts for one character.
+		 */
+
+		if (next_escaped) {
+			*dest++ = *src++;
+			next_escaped = False;
+			continue;
+		}
+
+		/*
+		 * Deal with single quote state. The
+		 * only thing we care about is exiting
+		 * this state.
+		 */
+
+		if (in_s_quote) {
+			if (*src == '\'') {
+				in_s_quote = False;
+			}
+			*dest++ = *src++;
+			continue;
+		}
+
+		/* 
+		 * Deal with double quote state. The most
+		 * complex state. We must cope with \, meaning
+		 * possibly escape next char (depending what it
+		 * is), ", meaning exit this state, and possibly
+		 * add an \ escape to any unprotected character
+		 * (listed in INSIDE_DQUOTE_LIST).
+		 */
+
+		if (in_d_quote) {
+			if (*src == '\\') {
+				/* 
+				 * Next character might be escaped.
+				 * We have to peek. Inside double
+				 * quotes only INSIDE_DQUOTE_LIST
+				 * characters are escaped by a \.
+				 */
+
+				char nextchar;
+
+				c = next_codepoint(&src[1], &c_size);
+				if (c == INVALID_CODEPOINT) {
+					SAFE_FREE(ret);
+					return NULL;
+				}
+				if (c_size > 1) {
+					/*
+					 * Don't escape the next char.
+					 * Just copy the \.
+					 */
+					*dest++ = *src++;
+					continue;
+				}
+
+				nextchar = src[1];
+
+				if (nextchar && strchr(INSIDE_DQUOTE_LIST, (int)nextchar)) {
+					next_escaped = True;
+				}
+				*dest++ = *src++;
+				continue;
+			}
+
+			if (*src == '\"') {
+				/* Exit double quote state. */
+				in_d_quote = False;
+				*dest++ = *src++;
+				continue;
+			}
+
+			/*
+			 * We know the character isn't \ or ",
+			 * so escape it if it's any of the other
+			 * possible unprotected characters.
+			 */
+
+	       		if (strchr(INSIDE_DQUOTE_LIST, (int)*src)) {
+				*dest++ = '\\';
+			}
+			*dest++ = *src++;
+			continue;
+		}
+
+		/* 
+		 * From here to the end of the loop we're
+		 * not in the single or double quote state.
+		 */
+
+		if (*src == '\\') {
+			/* Next character must be escaped. */
+			next_escaped = True;
+			*dest++ = *src++;
+			continue;
+		}
+
+		if (*src == '\'') {
+			/* Go into single quote state. */
+			in_s_quote = True;
+			*dest++ = *src++;
+			continue;
+		}
+
+		if (*src == '\"') {
+			/* Go into double quote state. */
+			in_d_quote = True;
+			*dest++ = *src++;
+			continue;
+		}
+
+		/* Check if we need to escape the character. */
+
+	       	if (!strchr(INCLUDE_LIST, (int)*src)) {
+			*dest++ = '\\';
+		}
+		*dest++ = *src++;
+	}
+	*dest++ = '\0';
+	return ret;
+}
diff --git a/source/printing/print_generic.c b/source/printing/print_generic.c
index 1e55f712c53..66965809700 100644
--- a/source/printing/print_generic.c
+++ b/source/printing/print_generic.c
@@ -64,7 +64,7 @@ static int print_run_command(int snum, const char* printername, BOOL do_sub,
 				      current_user_info.domain,
 				      syscmd, sizeof(syscmd));
 		
-	ret = smbrun(syscmd,outfd);
+	ret = smbrun_no_sanitize(syscmd,outfd);
 
 	DEBUG(3,("Running the command `%s' gave %d\n",syscmd,ret));
 
diff --git a/source/rpc_parse/parse_dfs.c b/source/rpc_parse/parse_dfs.c
index 118429e7d2f..88d6135a838 100644
--- a/source/rpc_parse/parse_dfs.c
+++ b/source/rpc_parse/parse_dfs.c
@@ -325,9 +325,13 @@ BOOL netdfs_io_dfs_Info3_d(const char *desc, NETDFS_DFS_INFO3 *v, prs_struct *ps
 			return False;
 		
 		if (UNMARSHALLING(ps)) {
-			v->stores = (void *)PRS_ALLOC_MEM_VOID(ps,sizeof(*v->stores)*v->num_stores);
-			if (!v->stores) {
-				return False;
+			if (v->num_stores) {
+				v->stores = PRS_ALLOC_MEM(ps,NETDFS_DFS_STORAGEINFO,v->num_stores);
+				if (!v->stores) {
+					return False;
+				}
+			} else {
+				v->stores = NULL;
 			}
 		}
 		for (i_stores_1=0; i_stores_1<v->num_stores;i_stores_1++) {
@@ -450,9 +454,13 @@ BOOL netdfs_io_dfs_Info4_d(const char *desc, NETDFS_DFS_INFO4 *v, prs_struct *ps
 			return False;
 		
 		if (UNMARSHALLING(ps)) {
-			v->stores = (void *)PRS_ALLOC_MEM_VOID(ps,sizeof(*v->stores)*v->num_stores);
-			if (!v->stores) {
-				return False;
+			if (v->num_stores) {
+				v->stores = PRS_ALLOC_MEM(ps,NETDFS_DFS_STORAGEINFO,v->num_stores);
+				if (!v->stores) {
+					return False;
+				}
+			} else {
+				v->stores = NULL;
 			}
 		}
 		for (i_stores_1=0; i_stores_1<v->num_stores;i_stores_1++) {
@@ -926,9 +934,13 @@ BOOL netdfs_io_dfs_EnumArray1_d(const char *desc, NETDFS_DFS_ENUMARRAY1 *v, prs_
 			return False;
 		
 		if (UNMARSHALLING(ps)) {
-			v->s = (void *)PRS_ALLOC_MEM_VOID(ps,sizeof(*v->s)*v->count);
-			if (!v->s) {
-				return False;
+			if (v->count) {
+				v->s = PRS_ALLOC_MEM(ps,NETDFS_DFS_INFO1,v->count);
+				if (!v->s) {
+					return False;
+				}
+			} else {
+				v->s = NULL;
 			}
 		}
 		for (i_s_1=0; i_s_1<v->count;i_s_1++) {
@@ -995,9 +1007,13 @@ BOOL netdfs_io_dfs_EnumArray2_d(const char *desc, NETDFS_DFS_ENUMARRAY2 *v, prs_
 			return False;
 		
 		if (UNMARSHALLING(ps)) {
-			v->s = (void *)PRS_ALLOC_MEM_VOID(ps,sizeof(*v->s)*v->count);
-			if (!v->s) {
-				return False;
+			if (v->count) {
+				v->s = PRS_ALLOC_MEM(ps,NETDFS_DFS_INFO2,v->count);
+				if (!v->s) {
+					return False;
+				}
+			} else {
+				v->s = NULL;
 			}
 		}
 		for (i_s_1=0; i_s_1<v->count;i_s_1++) {
@@ -1064,9 +1080,13 @@ BOOL netdfs_io_dfs_EnumArray3_d(const char *desc, NETDFS_DFS_ENUMARRAY3 *v, prs_
 			return False;
 		
 		if (UNMARSHALLING(ps)) {
-			v->s = (void *)PRS_ALLOC_MEM_VOID(ps,sizeof(*v->s)*v->count);
-			if (!v->s) {
-				return False;
+			if (v->count) {
+				v->s = PRS_ALLOC_MEM(ps,NETDFS_DFS_INFO3,v->count);
+				if (!v->s) {
+					return False;
+				}
+			} else {
+				v->s = NULL;
 			}
 		}
 		for (i_s_1=0; i_s_1<v->count;i_s_1++) {
@@ -1133,9 +1153,13 @@ BOOL netdfs_io_dfs_EnumArray4_d(const char *desc, NETDFS_DFS_ENUMARRAY4 *v, prs_
 			return False;
 		
 		if (UNMARSHALLING(ps)) {
-			v->s = (void *)PRS_ALLOC_MEM_VOID(ps,sizeof(*v->s)*v->count);
-			if (!v->s) {
-				return False;
+			if (v->count) {
+				v->s = PRS_ALLOC_MEM(ps,NETDFS_DFS_INFO4,v->count);
+				if (!v->s) {
+					return False;
+				}
+			} else {
+				v->s = NULL;
 			}
 		}
 		for (i_s_1=0; i_s_1<v->count;i_s_1++) {
@@ -1202,9 +1226,13 @@ BOOL netdfs_io_dfs_EnumArray200_d(const char *desc, NETDFS_DFS_ENUMARRAY200 *v,
 			return False;
 		
 		if (UNMARSHALLING(ps)) {
-			v->s = (void *)PRS_ALLOC_MEM_VOID(ps,sizeof(*v->s)*v->count);
-			if (!v->s) {
-				return False;
+			if (v->count) {
+				v->s = PRS_ALLOC_MEM(ps,NETDFS_DFS_INFO200,v->count);
+				if (!v->s) {
+					return False;
+				}
+			} else {
+				v->s = NULL;
 			}
 		}
 		for (i_s_1=0; i_s_1<v->count;i_s_1++) {
@@ -1271,9 +1299,13 @@ BOOL netdfs_io_dfs_EnumArray300_d(const char *desc, NETDFS_DFS_ENUMARRAY300 *v,
 			return False;
 		
 		if (UNMARSHALLING(ps)) {
-			v->s = (void *)PRS_ALLOC_MEM_VOID(ps,sizeof(*v->s)*v->count);
-			if (!v->s) {
-				return False;
+			if (v->count) {
+				v->s = PRS_ALLOC_MEM(ps,NETDFS_DFS_INFO300,v->count);
+				if (!v->s) {
+					return False;
+				}
+			} else {
+				v->s = NULL;
 			}
 		}
 		for (i_s_1=0; i_s_1<v->count;i_s_1++) {
diff --git a/source/rpc_parse/parse_lsa.c b/source/rpc_parse/parse_lsa.c
index ea249dc5600..06ccec4ab34 100644
--- a/source/rpc_parse/parse_lsa.c
+++ b/source/rpc_parse/parse_lsa.c
@@ -1356,12 +1356,17 @@ static BOOL lsa_io_trans_names(const char *desc, LSA_TRANS_NAME_ENUM *trn,
 			       &trn->num_entries2))
 			return False;
 
+		if (trn->num_entries2 != trn->num_entries) {
+			/* RPC fault */
+			return False;
+		}
+
 		if (UNMARSHALLING(ps)) {
-			if ((trn->name = PRS_ALLOC_MEM(ps, LSA_TRANS_NAME, trn->num_entries)) == NULL) {
+			if ((trn->name = PRS_ALLOC_MEM(ps, LSA_TRANS_NAME, trn->num_entries2)) == NULL) {
 				return False;
 			}
 
-			if ((trn->uni_name = PRS_ALLOC_MEM(ps, UNISTR2, trn->num_entries)) == NULL) {
+			if ((trn->uni_name = PRS_ALLOC_MEM(ps, UNISTR2, trn->num_entries2)) == NULL) {
 				return False;
 			}
 		}
@@ -1413,12 +1418,17 @@ static BOOL lsa_io_trans_names2(const char *desc, LSA_TRANS_NAME_ENUM2 *trn,
 			       &trn->num_entries2))
 			return False;
 
+		if (trn->num_entries2 != trn->num_entries) {
+			/* RPC fault */
+			return False;
+		}
+
 		if (UNMARSHALLING(ps)) {
-			if ((trn->name = PRS_ALLOC_MEM(ps, LSA_TRANS_NAME2, trn->num_entries)) == NULL) {
+			if ((trn->name = PRS_ALLOC_MEM(ps, LSA_TRANS_NAME2, trn->num_entries2)) == NULL) {
 				return False;
 			}
 
-			if ((trn->uni_name = PRS_ALLOC_MEM(ps, UNISTR2, trn->num_entries)) == NULL) {
+			if ((trn->uni_name = PRS_ALLOC_MEM(ps, UNISTR2, trn->num_entries2)) == NULL) {
 				return False;
 			}
 		}
@@ -2771,7 +2781,7 @@ static BOOL lsa_io_luid_attr(const char *desc, LUID_ATTR *out, prs_struct *ps, i
 
 static BOOL lsa_io_privilege_set(const char *desc, PRIVILEGE_SET *out, prs_struct *ps, int depth)
 {
-	uint32 i;
+	uint32 i, dummy;
 
 	prs_debug(ps, depth, desc, "lsa_io_privilege_set");
 	depth++;
@@ -2779,7 +2789,7 @@ static BOOL lsa_io_privilege_set(const char *desc, PRIVILEGE_SET *out, prs_struc
 	if(!prs_align(ps))
 		return False;
  
-	if(!prs_uint32("count", ps, depth, &out->count))
+	if(!prs_uint32("count", ps, depth, &dummy))
 		return False;
 	if(!prs_uint32("control", ps, depth, &out->control))
 		return False;
diff --git a/source/rpc_parse/parse_prs.c b/source/rpc_parse/parse_prs.c
index 2a5daac2e6e..868a604ffe5 100644
--- a/source/rpc_parse/parse_prs.c
+++ b/source/rpc_parse/parse_prs.c
@@ -644,7 +644,7 @@ BOOL prs_pointer( const char *name, prs_struct *ps, int depth,
 		return True;
 
 	if (UNMARSHALLING(ps)) {
-		if ( !(*data = PRS_ALLOC_MEM_VOID(ps, data_size)) )
+		if ( !(*data = PRS_ALLOC_MEM(ps, char, data_size)) )
 			return False;
 	}
 
diff --git a/source/rpc_parse/parse_sec.c b/source/rpc_parse/parse_sec.c
index 76583605939..15c6d7f1657 100644
--- a/source/rpc_parse/parse_sec.c
+++ b/source/rpc_parse/parse_sec.c
@@ -147,13 +147,12 @@ BOOL sec_io_acl(const char *desc, SEC_ACL **ppsa, prs_struct *ps, int depth)
 		return False;
 
 	if (UNMARSHALLING(ps)) {
-		/*
-		 * Even if the num_aces is zero, allocate memory as there's a difference
-		 * between a non-present DACL (allow all access) and a DACL with no ACE's
-		 * (allow no access).
-		 */
-		if((psa->aces = PRS_ALLOC_MEM(ps, SEC_ACE, psa->num_aces+1)) == NULL)
-			return False;
+		if (psa->num_aces) {
+			if((psa->aces = PRS_ALLOC_MEM(ps, SEC_ACE, psa->num_aces)) == NULL)
+				return False;
+		} else {
+			psa->aces = NULL;
+		}
 	}
 
 	for (i = 0; i < psa->num_aces; i++) {
diff --git a/source/rpc_parse/parse_spoolss.c b/source/rpc_parse/parse_spoolss.c
index ae82f9c1164..060b53f44a9 100644
--- a/source/rpc_parse/parse_spoolss.c
+++ b/source/rpc_parse/parse_spoolss.c
@@ -230,6 +230,10 @@ static BOOL smb_io_notify_option_type_data(const char *desc, SPOOL_NOTIFY_OPTION
 	if (type->count2 != type->count)
 		DEBUG(4,("What a mess, count was %x now is %x !\n", type->count, type->count2));
 
+	if (type->count2 > MAX_NOTIFY_TYPE_FOR_NOW) {
+		return False;
+	}
+
 	/* parse the option type data */
 	for(i=0;i<type->count2;i++)
 		if(!prs_uint16("fields",ps,depth,&type->fields[i]))