CVE-2013-4408:s3:Ensure LookupSids replies arrays are range checked.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=10185

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Jeremy Allison <jra@samba.org>

8 files changed, 110 insertions, 8 deletions

diff --git a/nsswitch/libwbclient/wbc_sid.c b/nsswitch/libwbclient/wbc_sid.c
index 6df8a3c375f..35319c536e7 100644
--- a/nsswitch/libwbclient/wbc_sid.c
+++ b/nsswitch/libwbclient/wbc_sid.c
@@ -421,6 +421,13 @@ wbcErr wbcLookupSids(const struct wbcDomainSid *sids, int num_sids,
 	for (i=0; i<num_names; i++) {
 
 		names[i].domain_index = strtoul(p, &q, 10);
+		if (names[i].domain_index < 0) {
+			goto wbc_err_invalid;
+		}
+		if (names[i].domain_index >= num_domains) {
+			goto wbc_err_invalid;
+		}
+
 		if (*q != ' ') {
 			goto wbc_err_invalid;
 		}
diff --git a/nsswitch/wbinfo.c b/nsswitch/wbinfo.c
index 9d25f59b8c0..8b822d7c708 100644
--- a/nsswitch/wbinfo.c
+++ b/nsswitch/wbinfo.c
@@ -1380,11 +1380,28 @@ static bool wbinfo_lookup_sids(const char *arg)
 	}
 
 	for (i=0; i<num_sids; i++) {
+		const char *domain = NULL;
+
 		wbcSidToStringBuf(&sids[i], sidstr, sizeof(sidstr));
 
-		d_printf("%s -> %s\\%s %d\n", sidstr,
-			 domains[names[i].domain_index].short_name,
-			 names[i].name, names[i].type);
+		if (names[i].domain_index >= num_domains) {
+			domain = "<none>";
+		} else if (names[i].domain_index < 0) {
+			domain = "<none>";
+		} else {
+			domain = domains[names[i].domain_index].short_name;
+		}
+
+		if (names[i].type == WBC_SID_NAME_DOMAIN) {
+			d_printf("%s -> %s %d\n", sidstr,
+				domain,
+				names[i].type);
+		} else {
+			d_printf("%s -> %s%c%s %d\n", sidstr,
+				domain,
+				winbind_separator(),
+				names[i].name, names[i].type);
+		}
 	}
 	return true;
 }
diff --git a/source3/rpc_client/cli_lsarpc.c b/source3/rpc_client/cli_lsarpc.c
index 330774d9d3d..e7e8236012f 100644
--- a/source3/rpc_client/cli_lsarpc.c
+++ b/source3/rpc_client/cli_lsarpc.c
@@ -279,11 +279,26 @@ static NTSTATUS dcerpc_lsa_lookup_sids_noalloc(struct dcerpc_binding_handle *h,
 
 	for (i = 0; i < num_sids; i++) {
 		const char *name, *dom_name;
-		uint32_t dom_idx = lsa_names.names[i].sid_index;
+		uint32_t dom_idx;
+
+		if (i >= lsa_names.count) {
+			*presult = NT_STATUS_INVALID_NETWORK_RESPONSE;
+			return status;
+		}
+
+		dom_idx = lsa_names.names[i].sid_index;
 
 		/* Translate optimised name through domain index array */
 
 		if (dom_idx != 0xffffffff) {
+			if (ref_domains == NULL) {
+				*presult = NT_STATUS_INVALID_NETWORK_RESPONSE;
+				return status;
+			}
+			if (dom_idx >= ref_domains->count) {
+				*presult = NT_STATUS_INVALID_NETWORK_RESPONSE;
+				return status;
+			}
 
 			dom_name = ref_domains->domains[dom_idx].name.string;
 			name = lsa_names.names[i].name.string;
diff --git a/source3/rpcclient/cmd_lsarpc.c b/source3/rpcclient/cmd_lsarpc.c
index 3aaef5cac21..9893a7c6d30 100644
--- a/source3/rpcclient/cmd_lsarpc.c
+++ b/source3/rpcclient/cmd_lsarpc.c
@@ -450,7 +450,7 @@ static NTSTATUS cmd_lsa_lookup_sids3(struct rpc_pipe_client *cli,
 	NTSTATUS status = NT_STATUS_UNSUCCESSFUL, result;
 	int i;
 	struct lsa_SidArray sids;
-	struct lsa_RefDomainList *domains;
+	struct lsa_RefDomainList *domains = NULL;
 	struct lsa_TransNameArray2 names;
 	uint32_t count = 0;
 	struct dcerpc_binding_handle *b = cli->binding_handle;
@@ -506,9 +506,12 @@ static NTSTATUS cmd_lsa_lookup_sids3(struct rpc_pipe_client *cli,
 
 	/* Print results */
 
-	for (i = 0; i < count; i++) {
+	for (i = 0; i < names.count; i++) {
 		fstring sid_str;
 
+		if (i >= sids.num_sids) {
+			break;
+		}
 		sid_to_fstring(sid_str, sids.sids[i].sid);
 		printf("%s %s (%d)\n", sid_str,
 		       names.names[i].name.string,
diff --git a/source3/winbindd/wb_lookupsids.c b/source3/winbindd/wb_lookupsids.c
index 2fd735d9dd3..32f646cbf26 100644
--- a/source3/winbindd/wb_lookupsids.c
+++ b/source3/winbindd/wb_lookupsids.c
@@ -402,6 +402,9 @@ static bool wb_lookupsids_move_name(struct lsa_RefDomainList *src_domains,
 	uint32_t src_domain_index, dst_domain_index;
 
 	src_domain_index = src_name->sid_index;
+	if (src_domain_index >= src_domains->count) {
+		return false;
+	}
 	src_domain = &src_domains->domains[src_domain_index];
 
 	if (!wb_lookupsids_find_dom_idx(
diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c
index 9a95e57d8ab..ad4ce443a52 100644
--- a/source3/winbindd/winbindd_rpc.c
+++ b/source3/winbindd/winbindd_rpc.c
@@ -1060,6 +1060,10 @@ static NTSTATUS rpc_try_lookup_sids3(TALLOC_CTX *mem_ctx,
 	if (NT_STATUS_IS_ERR(result)) {
 		return result;
 	}
+	if (sids->num_sids != lsa_names2.count) {
+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
+	}
+
 	names = TALLOC_ZERO_P(mem_ctx, struct lsa_TransNameArray);
 	if (names == NULL) {
 		return NT_STATUS_NO_MEMORY;
@@ -1075,6 +1079,16 @@ static NTSTATUS rpc_try_lookup_sids3(TALLOC_CTX *mem_ctx,
 		names->names[i].name.string = talloc_move(
 			names->names, &lsa_names2.names[i].name.string);
 		names->names[i].sid_index = lsa_names2.names[i].sid_index;
+
+		if (names->names[i].sid_index == UINT32_MAX) {
+			continue;
+		}
+		if ((*pdomains) == NULL) {
+			return NT_STATUS_INVALID_NETWORK_RESPONSE;
+		}
+		if (names->names[i].sid_index >= (*pdomains)->count) {
+			return NT_STATUS_INVALID_NETWORK_RESPONSE;
+		}
 	}
 	*pnames = names;
 	return result;
@@ -1090,6 +1104,7 @@ NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx,
 	struct rpc_pipe_client *cli = NULL;
 	struct policy_handle lsa_policy;
 	uint32_t count;
+	uint32_t i;
 	NTSTATUS status, result;
 
 	status = cm_connect_lsat(domain, mem_ctx, &cli, &lsa_policy);
@@ -1116,6 +1131,23 @@ NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx,
 	if (NT_STATUS_IS_ERR(result)) {
 		return result;
 	}
+
+	if (sids->num_sids != names->count) {
+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
+	}
+
+	for (i=0; i < names->count; i++) {
+		if (names->names[i].sid_index == UINT32_MAX) {
+			continue;
+		}
+		if ((*pdomains) == NULL) {
+			return NT_STATUS_INVALID_NETWORK_RESPONSE;
+		}
+		if (names->names[i].sid_index >= (*pdomains)->count) {
+			return NT_STATUS_INVALID_NETWORK_RESPONSE;
+		}
+	}
+
 	*pnames = names;
 	return result;
 }
diff --git a/source4/libcli/util/clilsa.c b/source4/libcli/util/clilsa.c
index 4cfdf937259..d705abdca49 100644
--- a/source4/libcli/util/clilsa.c
+++ b/source4/libcli/util/clilsa.c
@@ -254,7 +254,21 @@ NTSTATUS smblsa_lookup_sid(struct smbcli_state *cli,
 	}
 	if (names.count != 1) {
 		talloc_free(mem_ctx2);
-		return NT_STATUS_UNSUCCESSFUL;
+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
+	}
+	if (domains == NULL) {
+		talloc_free(mem_ctx2);
+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
+	}
+	if (domains->count != 1) {
+		talloc_free(mem_ctx2);
+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
+	}
+	if (names.names[0].sid_index != UINT32_MAX &&
+	    names.names[0].sid_index >= domains->count)
+	{
+		talloc_free(mem_ctx2);
+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
 	}
 
 	(*name) = talloc_asprintf(mem_ctx, "%s\\%s", 
diff --git a/source4/winbind/wb_async_helpers.c b/source4/winbind/wb_async_helpers.c
index 5d530ca149b..01ff4a42e99 100644
--- a/source4/winbind/wb_async_helpers.c
+++ b/source4/winbind/wb_async_helpers.c
@@ -120,6 +120,12 @@ static void lsa_lookupsids_recv_names(struct tevent_req *subreq)
 		return;
 	}
 
+	if (state->names.count != state->num_sids) {
+		composite_error(state->ctx,
+				NT_STATUS_INVALID_NETWORK_RESPONSE);
+		return;
+	}
+
 	state->result = talloc_array(state, struct wb_sid_object *,
 				     state->num_sids);
 	if (composite_nomem(state->result, state->ctx)) return;
@@ -140,9 +146,14 @@ static void lsa_lookupsids_recv_names(struct tevent_req *subreq)
 			continue;
 		}
 
+		if (domains == NULL) {
+			composite_error(state->ctx,
+					NT_STATUS_INVALID_NETWORK_RESPONSE);
+			return;
+		}
 		if (name->sid_index >= domains->count) {
 			composite_error(state->ctx,
-					NT_STATUS_INVALID_PARAMETER);
+					NT_STATUS_INVALID_NETWORK_RESPONSE);
 			return;
 		}