Fix the %m security bug again - and try to make it harder to reintroduce in

future.

This moves us from fstrcpy() and global variables to 'get' and 'set' functions.

In particular, the 'set' function sainity-checks the input, in the same way as
we always have.

Andrew Bartlett
(This used to be commit e57a896f06b16fe7e336e1ae63a0c9e4cc75fd36)

6 files changed, 60 insertions, 34 deletions

diff --git a/source3/client/smbmount.c b/source3/client/smbmount.c
index ad050063ec..0db990e8bd 100644
--- a/source3/client/smbmount.c
+++ b/source3/client/smbmount.c
@@ -29,7 +29,6 @@
 extern BOOL in_client;
 extern pstring user_socket_options;
 extern BOOL append_log;
-extern fstring remote_machine;
 
 static pstring credentials;
 static pstring my_netbios_name;
@@ -377,7 +376,7 @@ static void send_fs_socket(char *the_service, char *mount_point, struct cli_stat
 			}
 
 			/* here we are no longer interactive */
-			pstrcpy(remote_machine, "smbmount");	/* sneaky ... */
+			set_remote_machine_name("smbmount");	/* sneaky ... */
 			setup_logging("mount.smbfs", False);
 			append_log = True;
 			reopen_logs();
diff --git a/source3/lib/substitute.c b/source3/lib/substitute.c
index 6d96a1820f..c47b5914f1 100644
--- a/source3/lib/substitute.c
+++ b/source3/lib/substitute.c
@@ -28,6 +28,36 @@ fstring remote_proto="UNKNOWN";
 fstring remote_machine="";
 extern pstring global_myname;
 
+void set_local_machine_name(const char* local_name)
+{
+	fstring tmp_local_machine;
+
+	fstrcpy(tmp_local_machine,local_name);
+	trim_string(tmp_local_machine," "," ");
+	strlower(tmp_local_machine);
+	alpha_strcpy(local_machine,tmp_local_machine,SAFE_NETBIOS_CHARS,sizeof(local_machine)-1);
+}
+
+void set_remote_machine_name(const char* remote_name)
+{
+	fstring tmp_remote_machine;
+
+	fstrcpy(tmp_remote_machine,remote_name);
+	trim_string(tmp_remote_machine," "," ");
+	strlower(tmp_remote_machine);
+	alpha_strcpy(remote_machine,tmp_remote_machine,SAFE_NETBIOS_CHARS,sizeof(remote_machine)-1);
+}
+
+const char* get_remote_machine_name(void) 
+{
+	return remote_machine;
+}
+
+const char* get_local_machine_name(void) 
+{
+	return local_machine;
+}
+
 /*******************************************************************
  Given a pointer to a %$(NAME) expand it as an environment variable.
  Return the number of characters by which the pointer should be advanced.
@@ -188,14 +218,15 @@ static char *automount_path(const char *user_name)
  moved out to a separate function.
 *******************************************************************/
 
-static char *automount_server(const char *user_name)
+static const char *automount_server(const char *user_name)
 {
 	static pstring server_name;
+	const char *local_machine_name = get_local_machine_name(); 
 
 	/* use the local machine name as the default */
 	/* this will be the default if WITH_AUTOMOUNT is not used or fails */
-	if (*local_machine)
-		pstrcpy(server_name, local_machine);
+	if (local_machine_name && *local_machine_name)
+		pstrcpy(server_name, local_machine_name);
 	else
 		pstrcpy(server_name, global_myname);
 	
@@ -229,6 +260,7 @@ void standard_sub_basic(const char *smb_name, char *str,size_t len)
 	char *p, *s;
 	fstring pidstr;
 	struct passwd *pass;
+	const char *local_machine_name = get_local_machine_name();
 
 	for (s=str; (p=strchr_m(s, '%'));s=p) {
 		fstring tmp_str;
@@ -261,8 +293,8 @@ void standard_sub_basic(const char *smb_name, char *str,size_t len)
 			string_sub(p,"%I", client_addr(),l);
 			break;
 		case 'L' : 
-			if (*local_machine)
-				string_sub(p,"%L", local_machine,l); 
+			if (local_machine_name && *local_machine_name)
+				string_sub(p,"%L", local_machine_name,l); 
 			else
 				string_sub(p,"%L", global_myname,l); 
 			break;
@@ -286,7 +318,7 @@ void standard_sub_basic(const char *smb_name, char *str,size_t len)
 			string_sub(p,"%h", myhostname(),l);
 			break;
 		case 'm' :
-			string_sub(p,"%m", remote_machine,l);
+			string_sub(p,"%m", get_remote_machine_name(),l);
 			break;
 		case 'v' :
 			string_sub(p,"%v", VERSION,l);
@@ -381,6 +413,7 @@ char *alloc_sub_basic(const char *smb_name, const char *str)
 	char *b, *p, *s, *t, *r, *a_string;
 	fstring pidstr;
 	struct passwd *pass;
+	const char *local_machine_name = get_local_machine_name();
 
 	a_string = strdup(str);
 	if (a_string == NULL) {
@@ -415,8 +448,8 @@ char *alloc_sub_basic(const char *smb_name, const char *str)
 			t = realloc_string_sub(t, "%I", client_addr());
 			break;
 		case 'L' : 
-			if (*local_machine)
-				t = realloc_string_sub(t, "%L", local_machine); 
+			if (local_machine_name && *local_machine_name)
+				t = realloc_string_sub(t, "%L", local_machine_name); 
 			else
 				t = realloc_string_sub(t, "%L", global_myname); 
 			break;
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index ba0e15bd4e..a4ed770f31 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -53,7 +53,6 @@ int reply_special(char *inbuf,char *outbuf)
 	int msg_flags = CVAL(inbuf,1);
 	pstring name1,name2;
 
-	extern fstring local_machine;
 	int len;
 	char name_type = 0;
 	
@@ -84,24 +83,19 @@ int reply_special(char *inbuf,char *outbuf)
 		DEBUG(2,("netbios connect: name1=%s name2=%s\n",
 			 name1,name2));      
 
-		fstrcpy(remote_machine,name2);
-		remote_machine[15] = 0;
-		trim_string(remote_machine," "," ");
-		strlower(remote_machine);
-		alpha_strcpy(remote_machine,remote_machine,SAFE_NETBIOS_CHARS,sizeof(remote_machine)-1);
+		name1[15] = 0;
 
-		fstrcpy(local_machine,name1);
-		len = strlen(local_machine);
+		len = strlen(name2);
 		if (len == 16) {
-			name_type = local_machine[15];
-			local_machine[15] = 0;
+			name_type = name2[15];
+			name2[15] = 0;
 		}
-		trim_string(local_machine," "," ");
-		strlower(local_machine);
-		alpha_strcpy(local_machine,local_machine,SAFE_NETBIOS_CHARS,sizeof(local_machine)-1);
+
+		set_local_machine_name(name1);
+		set_remote_machine_name(name2);
 
 		DEBUG(2,("netbios connect: local=%s remote=%s\n",
-			local_machine, remote_machine ));
+			get_local_machine_name(), get_remote_machine_name() ));
 
 		if (name_type == 'R') {
 			/* We are being asked for a pathworks session --- 
diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index d173fec00e..45295896e8 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -38,8 +38,6 @@ extern pstring user_socket_options;
 extern int dcelogin_atmost_once;
 #endif /* WITH_DFS */
 
-extern fstring remote_machine;
-
 /* really we should have a top level context structure that has the
    client file descriptor as an element. That would require a major rewrite :(
 
@@ -366,7 +364,7 @@ static BOOL open_sockets_smbd(BOOL is_daemon,const char *smb_ports)
 				
 				/* this is needed so that we get decent entries
 				   in smbstatus for port 445 connects */
-				fstrcpy(remote_machine, get_socket_addr(smbd_server_fd()));
+				set_remote_machine_name(get_socket_addr(smbd_server_fd()));
 				
 				/* Reset global variables in util.c so
 				   that client substitutions will be
@@ -742,7 +740,7 @@ static void usage(char *pname)
 		lp_set_logfile(logfile);
 	}
 
-	fstrcpy(remote_machine, "smbd");
+	set_remote_machine_name("smbd");
 
 	setup_logging(argv[0],interactive);
 
diff --git a/source3/smbd/sesssetup.c b/source3/smbd/sesssetup.c
index 2e9e54b8d9..f6d536f301 100644
--- a/source3/smbd/sesssetup.c
+++ b/source3/smbd/sesssetup.c
@@ -385,7 +385,6 @@ static int reply_spnego_auth(connection_struct *conn, char *inbuf, char *outbuf,
 	uint32 auth_flags = AUTH_FLAG_NONE;
 	auth_usersupplied_info *user_info = NULL;
 	auth_serversupplied_info *server_info = NULL;
-	extern fstring remote_machine;
 
 	/* we must have setup the auth context by now */
 	if (!ntlmssp_auth_context) {
@@ -422,7 +421,9 @@ static int reply_spnego_auth(connection_struct *conn, char *inbuf, char *outbuf,
 
 	/* the client has given us its machine name (which we otherwise would not get on port 445).
 	   we need to possibly reload smb.conf if smb.conf includes depend on the machine name */
-	fstrcpy(remote_machine, machine);
+
+	set_remote_machine_name(machine);
+
 	reload_services(True);
 
 #if 0
diff --git a/source3/utils/testparm.c b/source3/utils/testparm.c
index 1d48249a75..3086019467 100644
--- a/source3/utils/testparm.c
+++ b/source3/utils/testparm.c
@@ -175,7 +175,6 @@ int main(int argc, char *argv[])
 {
 	extern char *optarg;
 	extern int optind;
-	extern fstring local_machine;
 	const char *config_file = dyn_CONFIGFILE;
 	int s;
 	static BOOL silent_mode = False;
@@ -183,7 +182,7 @@ int main(int argc, char *argv[])
 	int opt;
 	poptContext pc;
 	static char *term_code = "";
-	static char *new_local_machine = local_machine;
+	static char *new_local_machine = NULL;
 	const char *cname;
 	const char *caddr;
 
@@ -207,8 +206,10 @@ int main(int argc, char *argv[])
 
 	cname = poptGetArg(pc);
 	caddr = poptGetArg(pc);
-
-	fstrcpy(local_machine,new_local_machine);
+	
+	if (new_local_machine) {
+		set_local_machine_name(new_local_machine);
+	}
 
 	dbf = x_stdout;
 	DEBUGLEVEL = 2;