[patch] check for dangerous substitutions

[Ihar Viarheichyk]

Hello.

This is an updatet alpha_strcpy patch. Check added for 'add share
command' and 'change share command'. Unfortunately this fix does not
allow non-latin share and path names and comments, and can be considered
as partial soluton intendend to fix a possible security hole.

-- 
Igor Vergeichik
ICQ 47298730

-------------- next part --------------
diff -ruNk.orig samba.HEAD/source/lib/util_str.c.orig samba.HEAD/source/lib/util_str.c
--- samba.HEAD/source/lib/util_str.c.orig	Tue Feb 19 13:00:49 2002
+++ samba.HEAD/source/lib/util_str.c	Tue Feb 19 16:26:15 2002
@@ -655,15 +655,24 @@
 void string_sub(char *s,const char *pattern,const char *insert, size_t len)
 {
 	char *p;
+	char *safe_insert=NULL;
 	ssize_t ls,lp,li, i;
 
-	if (!insert || !pattern || !s) return;
+	if (!insert || !pattern || !s ||!*pattern) return;
 
+	if(*pattern=='%' && strchr("mMDUSftRa", *(pattern+1))) {
+		safe_insert=strdup(insert);
+		if(!safe_insert) {
+			DEBUG(0, ("ERROR: can't allocate memory for string. Substitution failed"));
+			return;
+		}
+		alpha_strcpy(safe_insert, insert, ". _-$", strlen(safe_insert)+1);
+		insert=safe_insert;
+	}
+	
 	ls = (ssize_t)strlen(s);
 	lp = (ssize_t)strlen(pattern);
 	li = (ssize_t)strlen(insert);
-
-	if (!*pattern) return;
 	
 	while (lp <= ls && (p = strstr(s,pattern))) {
 		if (len && (ls + (li-lp) >= len)) {
@@ -694,6 +703,7 @@
 		s = p + li;
 		ls += (li-lp);
 	}
+	SAFE_FREE(safe_insert);
 }
 
 void fstring_sub(char *s,const char *pattern,const char *insert)
diff -ruNk.orig samba.HEAD/source/rpc_server/srv_reg_nt.c.orig samba.HEAD/source/rpc_server/srv_reg_nt.c
--- samba.HEAD/source/rpc_server/srv_reg_nt.c.orig	Tue Feb 19 13:15:25 2002
+++ samba.HEAD/source/rpc_server/srv_reg_nt.c	Tue Feb 19 15:47:12 2002
@@ -195,8 +195,6 @@
 	
 	/* message */
 	rpcstr_pull (message, unimsg.buffer, sizeof(message), unimsg.uni_str_len*2,0);
-		/* security check */
-	alpha_strcpy (chkmsg, message, NULL, sizeof(message));
 	/* timeout */
 	snprintf(timeout, sizeof(timeout), "%d", q_u->timeout);
 	/* reboot */
@@ -208,7 +206,7 @@
 
 	if(*shutdown_script) {
 		int shutdown_ret;
-		all_string_sub(shutdown_script, "%m", chkmsg, sizeof(shutdown_script));
+		string_sub(shutdown_script, "%m", chkmsg, sizeof(shutdown_script));
 		all_string_sub(shutdown_script, "%t", timeout, sizeof(shutdown_script));
 		all_string_sub(shutdown_script, "%r", r, sizeof(shutdown_script));
 		all_string_sub(shutdown_script, "%f", f, sizeof(shutdown_script));
diff -ruNk.orig samba.HEAD/source/rpc_server/srv_srvsvc_nt.c.orig samba.HEAD/source/rpc_server/srv_srvsvc_nt.c
--- samba.HEAD/source/rpc_server/srv_srvsvc_nt.c.orig	Thu Feb 21 15:27:02 2002
+++ samba.HEAD/source/rpc_server/srv_srvsvc_nt.c	Thu Feb 21 18:09:41 2002
@@ -1330,6 +1330,13 @@
 		if (!lp_change_share_cmd() || !*lp_change_share_cmd())
 			return NT_STATUS_ACCESS_DENIED;
 
+		/* Ensure share name, path and comment do not contain unsafe */
+		/* characters*/
+		/* This check breaks internationalzation (VIV)*/
+		alpha_strcpy(share_name, share_name, ". -$", sizeof(share_name));
+		alpha_strcpy(ptr, ptr, ". -$/", strlen(ptr)+1);
+		alpha_strcpy(comment, comment, ". -$", sizeof(comment));
+
 		slprintf(command, sizeof(command)-1, "%s \"%s\" \"%s\" \"%s\" \"%s\"",
 				lp_change_share_cmd(), dyn_CONFIGFILE, share_name, ptr, comment);
 
@@ -1446,6 +1453,11 @@
 	string_replace(share_name, '"', ' ');
 	string_replace(ptr, '"', ' ');
 	string_replace(comment, '"', ' ');
+	/* Ensure share name, path and comment do not contain unsafe characters*/
+	/* This check breaks internationalzation (VIV)*/
+	alpha_strcpy(share_name, share_name, ". -$", sizeof(share_name));
+	alpha_strcpy(ptr, ptr, ". -$/", strlen(ptr)+1);
+	alpha_strcpy(comment, comment, ". -$", sizeof(comment));
 
 	slprintf(command, sizeof(command)-1, "%s \"%s\" \"%s\" \"%s\" \"%s\"",
 			lp_add_share_cmd(), dyn_CONFIGFILE, share_name, ptr, comment);
diff -ruNk.orig samba.HEAD/source/smbd/message.c.orig samba.HEAD/source/smbd/message.c
--- samba.HEAD/source/smbd/message.c.orig	Tue Feb 19 15:50:35 2002
+++ samba.HEAD/source/smbd/message.c	Tue Feb 19 15:52:07 2002
@@ -83,8 +83,8 @@
       pstring s;
 
       pstrcpy(s,lp_msg_command());
-      pstring_sub(s,"%f",alpha_strcpy(alpha_msgfrom,msgfrom,NULL,sizeof(alpha_msgfrom)));
-      pstring_sub(s,"%t",alpha_strcpy(alpha_msgto,msgto,NULL,sizeof(alpha_msgto)));
+      pstring_sub(s,"%f",msgfrom);
+      pstring_sub(s,"%t",msgto);
       standard_sub_basic(current_user_info.smb_name, s);
       pstring_sub(s,"%s",name);
       smbrun(s,NULL);
diff -ruNk.orig samba.HEAD/source/smbd/sesssetup.c.orig samba.HEAD/source/smbd/sesssetup.c
--- samba.HEAD/source/smbd/sesssetup.c.orig	Tue Feb 19 12:39:05 2002
+++ samba.HEAD/source/smbd/sesssetup.c	Tue Feb 19 15:46:05 2002
@@ -681,9 +681,6 @@
 			 domain,native_os,native_lanman));
 	}
 	
-	/* don't allow for weird usernames or domains */
-	alpha_strcpy(user, user, ". _-$", sizeof(user));
-	alpha_strcpy(domain, domain, ". _-", sizeof(domain));
 	if (strstr(user, "..") || strstr(domain,"..")) {
 		return ERROR_NT(NT_STATUS_LOGON_FAILURE);
 	}