Remove strlower_m() and strupper_m() from source4 and common code.

This function is problematic because a string may expand in size when
changed into upper or lower case.  This will then push characters off
the end of the string in the s3 implementation, or panic in the former
s4 implementation.

Andrew Bartlett

8 files changed, 74 insertions, 63 deletions

diff --git a/source4/auth/ntlmssp/ntlmssp_server.c b/source4/auth/ntlmssp/ntlmssp_server.c
index 9db3b560c1..cdae37262e 100644
--- a/source4/auth/ntlmssp/ntlmssp_server.c
+++ b/source4/auth/ntlmssp/ntlmssp_server.c
@@ -308,26 +308,26 @@ NTSTATUS gensec_ntlmssp_server_start(struct gensec_security *gensec_security)
 	ntlmssp_state->server.netbios_domain = lpcfg_workgroup(gensec_security->settings->lp_ctx);
 
 	{
-		char dnsdomname[MAXHOSTNAMELEN], dnsname[MAXHOSTNAMELEN];
-
-		/* Find out the DNS domain name */
-		dnsdomname[0] = '\0';
-		safe_strcpy(dnsdomname, lpcfg_dnsdomain(gensec_security->settings->lp_ctx), sizeof(dnsdomname) - 1);
+		char *dnsdomain = lpcfg_dnsdomain(gensec_security->settings->lp_ctx);
+		char *dnsname, *lower_netbiosname;
+		lower_netbiosname = strlower_talloc(ntlmssp_state, ntlmssp_state->server.netbios_name);
 
 		/* Find out the DNS host name */
-		safe_strcpy(dnsname, ntlmssp_state->server.netbios_name, sizeof(dnsname) - 1);
-		if (dnsdomname[0] != '\0') {
-			safe_strcat(dnsname, ".", sizeof(dnsname) - 1);
-			safe_strcat(dnsname, dnsdomname, sizeof(dnsname) - 1);
+		if (dnsdomain && dnsdomain[0] != '\0') {
+			dnsname = talloc_asprintf(ntlmssp_state, "%s.%s",
+						  lower_netbiosname,
+						  dnsdomain);
+			talloc_free(lower_netbiosname);
+			ntlmssp_state->server.dns_name = dnsname;
+		} else {
+			ntlmssp_state->server.dns_name = lower_netbiosname;
 		}
-		strlower_m(dnsname);
 
-		ntlmssp_state->server.dns_name = talloc_strdup(ntlmssp_state,
-								      dnsname);
 		NT_STATUS_HAVE_NO_MEMORY(ntlmssp_state->server.dns_name);
 
-		ntlmssp_state->server.dns_domain = talloc_strdup(ntlmssp_state,
-								        dnsdomname);
+		ntlmssp_state->server.dns_domain
+			= talloc_strdup(ntlmssp_state,
+					lpcfg_dnsdomain(gensec_security->settings->lp_ctx));
 		NT_STATUS_HAVE_NO_MEMORY(ntlmssp_state->server.dns_domain);
 	}
 
diff --git a/source4/client/client.c b/source4/client/client.c
index c6c0088b3c..923a6f3a23 100644
--- a/source4/client/client.c
+++ b/source4/client/client.c
@@ -696,11 +696,12 @@ static int do_get(struct smbclient_context *ctx, char *rname, const char *p_lnam
 	char *lname;
 
 
-	lname = talloc_strdup(ctx, p_lname);
 	GetTimeOfDay(&tp_start);
 
 	if (ctx->lowercase) {
-		strlower(lname);
+		lname = strlower_talloc(ctx, p_lname);
+	} else {
+		lname = talloc_strdup(ctx, p_lname);
 	}
 
 	fnum = smbcli_open(ctx->cli->tree, rname, O_RDONLY, DENY_NONE);
@@ -884,13 +885,14 @@ static void do_mget(struct smbclient_context *ctx, struct clilist_file_info *fin
 
 	ctx->remote_cur_dir = talloc_asprintf_append_buffer(NULL, "%s\\", finfo->name);
 
-	l_fname = talloc_strdup(ctx, finfo->name);
-
-	string_replace(l_fname, '\\', '/');
 	if (ctx->lowercase) {
-		strlower(l_fname);
+		l_fname = strlower_talloc(ctx, finfo->name);
+	} else {
+		l_fname = talloc_strdup(ctx, finfo->name);
 	}
 	
+	string_replace(l_fname, '\\', '/');
+
 	if (!directory_exist(l_fname) &&
 	    mkdir(l_fname, 0777) != 0) {
 		d_printf("failed to create directory %s\n", l_fname);
diff --git a/source4/dsdb/samdb/ldb_modules/rootdse.c b/source4/dsdb/samdb/ldb_modules/rootdse.c
index b718900345..7bc27b46b4 100644
--- a/source4/dsdb/samdb/ldb_modules/rootdse.c
+++ b/source4/dsdb/samdb/ldb_modules/rootdse.c
@@ -222,11 +222,10 @@ static int rootdse_add_dynamic(struct ldb_module *module, struct ldb_message *ms
 					  struct loadparm_context);
 		char *ldap_service_name, *hostname;
 
-		hostname = talloc_strdup(msg, lpcfg_netbios_name(lp_ctx));
+		hostname = strlower_talloc(msg, lpcfg_netbios_name(lp_ctx));
 		if (hostname == NULL) {
 			goto failed;
 		}
-		strlower_m(hostname);
 
 		ldap_service_name = talloc_asprintf(msg, "%s:%s$@%s",
 						    samdb_forest_name(ldb, msg),
diff --git a/source4/ntvfs/cifs_posix_cli/svfs_util.c b/source4/ntvfs/cifs_posix_cli/svfs_util.c
index d8a7909390..128440c1ea 100644
--- a/source4/ntvfs/cifs_posix_cli/svfs_util.c
+++ b/source4/ntvfs/cifs_posix_cli/svfs_util.c
@@ -41,16 +41,15 @@ char *cifspsx_unix_path(struct ntvfs_module_context *ntvfs,
 {
 	struct cifspsx_private *p = ntvfs->private_data;
 	char *ret;
+	char *name_lower = strlower_talloc(p, name);
 
 	if (*name != '\\') {
-		ret = talloc_asprintf(req, "%s/%s", p->connectpath, name);
+		ret = talloc_asprintf(req, "%s/%s", p->connectpath, name_lower);
 	} else {
-		ret = talloc_asprintf(req, "%s%s", p->connectpath, name);
+		ret = talloc_asprintf(req, "%s%s", p->connectpath, name_lower);
 	}
 	all_string_sub(ret, "\\", "/", 0);
-
-	strlower(ret + strlen(p->connectpath));
-
+	talloc_free(name_lower);
 	return ret;
 }
 
@@ -85,9 +84,8 @@ struct cifspsx_dir *cifspsx_list_unix(TALLOC_CTX *mem_ctx, struct ntvfs_request
 	/* the wildcard pattern is the last part */
 	mask = p+1;
 
-	low_mask = talloc_strdup(mem_ctx, mask);
+	low_mask = strlower_talloc(mem_ctx, mask);
 	if (!low_mask) { return NULL; }
-	strlower(low_mask);
 
 	odir = opendir(dir->unix_dir);
 	if (!odir) { return NULL; }
@@ -102,9 +100,8 @@ struct cifspsx_dir *cifspsx_list_unix(TALLOC_CTX *mem_ctx, struct ntvfs_request
 			continue;
 		}
 
-		low_name = talloc_strdup(mem_ctx, dent->d_name);
+		low_name = strlower_talloc(mem_ctx, dent->d_name);
 		if (!low_name) { continue; }
-		strlower(low_name);
 
 		/* check it matches the wildcard pattern */
 		if (ms_fnmatch(low_mask, low_name, PROTOCOL_NT1) != 0) {
diff --git a/source4/ntvfs/simple/svfs_util.c b/source4/ntvfs/simple/svfs_util.c
index 2a01c2d5de..75261f7277 100644
--- a/source4/ntvfs/simple/svfs_util.c
+++ b/source4/ntvfs/simple/svfs_util.c
@@ -38,16 +38,15 @@ char *svfs_unix_path(struct ntvfs_module_context *ntvfs,
 {
 	struct svfs_private *p = ntvfs->private_data;
 	char *ret;
+	char *name_lower = strlower_talloc(p, name);
 
 	if (*name != '\\') {
-		ret = talloc_asprintf(req, "%s/%s", p->connectpath, name);
+		ret = talloc_asprintf(req, "%s/%s", p->connectpath, name_lower);
 	} else {
-		ret = talloc_asprintf(req, "%s%s", p->connectpath, name);
+		ret = talloc_asprintf(req, "%s%s", p->connectpath, name_lower);
 	}
 	all_string_sub(ret, "\\", "/", 0);
-
-	strlower(ret + strlen(p->connectpath));
-
+	talloc_free(name_lower);
 	return ret;
 }
 
@@ -82,9 +81,8 @@ struct svfs_dir *svfs_list_unix(TALLOC_CTX *mem_ctx, struct ntvfs_request *req,
 	/* the wildcard pattern is the last part */
 	mask = p+1;
 
-	low_mask = talloc_strdup(mem_ctx, mask);
+	low_mask = strlower_talloc(mem_ctx, mask);
 	if (!low_mask) { return NULL; }
-	strlower(low_mask);
 
 	odir = opendir(dir->unix_dir);
 	if (!odir) { return NULL; }
@@ -99,9 +97,8 @@ struct svfs_dir *svfs_list_unix(TALLOC_CTX *mem_ctx, struct ntvfs_request *req,
 			continue;
 		}
 
-		low_name = talloc_strdup(mem_ctx, dent->d_name);
+		low_name = strlower_talloc(mem_ctx, dent->d_name);
 		if (!low_name) { continue; }
-		strlower(low_name);
 
 		/* check it matches the wildcard pattern */
 		if (ms_fnmatch(low_mask, low_name, PROTOCOL_NT1) != 0) {
diff --git a/source4/param/loadparm.c b/source4/param/loadparm.c
index 0f60e4364b..fa00beee96 100644
--- a/source4/param/loadparm.c
+++ b/source4/param/loadparm.c
@@ -779,6 +779,7 @@ const char *lpcfg_get_parametric(struct loadparm_context *lp_ctx,
 			      struct loadparm_service *service,
 			      const char *type, const char *option)
 {
+	char *vfskey_tmp = NULL;
 	char *vfskey = NULL;
 	struct parmlist_entry *data;
 
@@ -787,13 +788,14 @@ const char *lpcfg_get_parametric(struct loadparm_context *lp_ctx,
 
 	data = (service == NULL ? lp_ctx->globals->param_opt : service->param_opt);
 
-	asprintf(&vfskey, "%s:%s", type, option);
-	if (vfskey == NULL) return NULL;
-	strlower(vfskey);
+	vfskey_tmp = talloc_asprintf(NULL, "%s:%s", type, option);
+	if (vfskey_tmp == NULL) return NULL;
+	vfskey = strlower_talloc(NULL, vfskey_tmp);
+	talloc_free(vfskey_tmp);
 
 	while (data) {
 		if (strcmp(data->key, vfskey) == 0) {
-			free(vfskey);
+			talloc_free(vfskey);
 			return data->value;
 		}
 		data = data->next;
@@ -805,13 +807,13 @@ const char *lpcfg_get_parametric(struct loadparm_context *lp_ctx,
 		for (data = lp_ctx->globals->param_opt; data;
 		     data = data->next) {
 			if (strcmp(data->key, vfskey) == 0) {
-				free(vfskey);
+				talloc_free(vfskey);
 				return data->value;
 			}
 		}
 	}
 
-	free(vfskey);
+	talloc_free(vfskey);
 
 	return NULL;
 }
@@ -1031,7 +1033,27 @@ static bool string_set(TALLOC_CTX *mem_ctx, char **dest, const char *src)
 
 	*dest = talloc_strdup(mem_ctx, src);
 	if ((*dest) == NULL) {
-		DEBUG(0,("Out of memory in string_init\n"));
+		DEBUG(0,("Out of memory in string_set\n"));
+		return false;
+	}
+
+	return true;
+}
+
+/**
+ * Set a string value, deallocating any existing space, and allocing the space
+ * for the string
+ */
+static bool string_set_upper(TALLOC_CTX *mem_ctx, char **dest, const char *src)
+{
+	talloc_free(*dest);
+
+	if (src == NULL)
+		src = "";
+
+	*dest = strupper_talloc(mem_ctx, src);
+	if ((*dest) == NULL) {
+		DEBUG(0,("Out of memory in string_set_upper\n"));
 		return false;
 	}
 
@@ -1292,10 +1314,9 @@ static void copy_service(struct loadparm_service *pserviceDest,
 					break;
 
 				case P_USTRING:
-					string_set(pserviceDest,
-						   (char **)dest_ptr,
-						   *(char **)src_ptr);
-					strupper(*(char **)dest_ptr);
+					string_set_upper(pserviceDest,
+							 (char **)dest_ptr,
+							 *(char **)src_ptr);
 					break;
 				case P_LIST:
 					*(const char ***)dest_ptr = (const char **)str_list_copy(pserviceDest, 
@@ -1574,11 +1595,9 @@ static bool lp_do_parameter_parametric(struct loadparm_context *lp_ctx,
 		pszParmName++;
 	}
 
-	name = strdup(pszParmName);
+	name = strlower_talloc(lp_ctx, pszParmName);
 	if (!name) return false;
 
-	strlower(name);
-
 	if (service == NULL) {
 		data = lp_ctx->globals->param_opt;
 		mem_ctx = lp_ctx->globals;
@@ -1594,13 +1613,14 @@ static bool lp_do_parameter_parametric(struct loadparm_context *lp_ctx,
 		if (strcmp(paramo->key, name) == 0) {
 			if ((paramo->priority & FLAG_CMDLINE) &&
 			    !(flags & FLAG_CMDLINE)) {
+				talloc_free(name);
 				return true;
 			}
 
 			talloc_free(paramo->value);
 			paramo->value = talloc_strdup(paramo, pszParmValue);
 			paramo->priority = flags;
-			free(name);
+			talloc_free(name);
 			return true;
 		}
 	}
@@ -1617,7 +1637,7 @@ static bool lp_do_parameter_parametric(struct loadparm_context *lp_ctx,
 		DLIST_ADD(service->param_opt, paramo);
 	}
 
-	free(name);
+	talloc_free(name);
 
 	return true;
 }
@@ -1713,8 +1733,7 @@ static bool set_variable(TALLOC_CTX *mem_ctx, int parmnum, void *parm_ptr,
 			break;
 
 		case P_USTRING:
-			string_set(mem_ctx, (char **)parm_ptr, pszParmValue);
-			strupper(*(char **)parm_ptr);
+			string_set_upper(mem_ctx, (char **)parm_ptr, pszParmValue);
 			break;
 
 		case P_ENUM:
diff --git a/source4/torture/masktest.c b/source4/torture/masktest.c
index 9c66291087..42665ab897 100644
--- a/source4/torture/masktest.c
+++ b/source4/torture/masktest.c
@@ -153,10 +153,8 @@ static void get_real_name(TALLOC_CTX *mem_ctx, struct smbcli_state *cli,
 			listfn, &state);
 
 	if (f_info_hit) {
-		*short_name = talloc_strdup(mem_ctx, last_hit.short_name);
-		strlower(*short_name);
-		*long_name = talloc_strdup(mem_ctx, last_hit.long_name);
-		strlower(*long_name);
+		*short_name = strlower_talloc(mem_ctx, last_hit.short_name);
+		*long_name = strlower_talloc(mem_ctx, last_hit.long_name);
 	}
 
 	if (*short_name == '\0') {
diff --git a/source4/torture/winbind/struct_based.c b/source4/torture/winbind/struct_based.c
index aeb81c972c..2bab94088a 100644
--- a/source4/torture/winbind/struct_based.c
+++ b/source4/torture/winbind/struct_based.c
@@ -914,7 +914,6 @@ static bool parse_domain_user(struct torture_context *torture,
 	fstrcpy(user, p+1);
 	fstrcpy(domain, domuser);
 	domain[PTR_DIFF(p, domuser)] = 0;
-	strupper_m(domain);
 
 	return true;
 }