CVE-2013-4408:s3:Ensure LookupRids() replies arrays are range checked.

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

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

6 files changed, 62 insertions, 5 deletions

diff --git a/source3/lib/netapi/group.c b/source3/lib/netapi/group.c
index 360640f5396..09a0f0b9f89 100644
--- a/source3/lib/netapi/group.c
+++ b/source3/lib/netapi/group.c
@@ -395,6 +395,14 @@ WERROR NetGroupDel_r(struct libnetapi_ctx *ctx,
 		werr = ntstatus_to_werror(result);
 		goto done;
 	}
+	if (names.count != rid_array->count) {
+		werr = WERR_BAD_NET_RESP;
+		goto done;
+	}
+	if (member_types.count != rid_array->count) {
+		werr = WERR_BAD_NET_RESP;
+		goto done;
+	}
 	}
 
 	for (i=0; i < rid_array->count; i++) {
@@ -1623,6 +1631,14 @@ WERROR NetGroupGetUsers_r(struct libnetapi_ctx *ctx,
 		werr = ntstatus_to_werror(result);
 		goto done;
 	}
+	if (names.count != rid_array->count) {
+		werr = WERR_BAD_NET_RESP;
+		goto done;
+	}
+	if (member_types.count != rid_array->count) {
+		werr = WERR_BAD_NET_RESP;
+		goto done;
+	}
 
 	for (i=0; i < names.count; i++) {
 
diff --git a/source3/lib/netapi/user.c b/source3/lib/netapi/user.c
index d16d2261ba1..7c21703c4e3 100644
--- a/source3/lib/netapi/user.c
+++ b/source3/lib/netapi/user.c
@@ -3113,6 +3113,14 @@ WERROR NetUserGetGroups_r(struct libnetapi_ctx *ctx,
 		werr = ntstatus_to_werror(result);
 		goto done;
 	}
+	if (names.count != rid_array->count) {
+		werr = WERR_BAD_NET_RESP;
+		goto done;
+	}
+	if (types.count != rid_array->count) {
+		werr = WERR_BAD_NET_RESP;
+		goto done;
+	}
 
 	for (i=0; i < names.count; i++) {
 		status = add_GROUP_USERS_INFO_X_buffer(ctx,
@@ -3716,6 +3724,14 @@ WERROR NetUserGetLocalGroups_r(struct libnetapi_ctx *ctx,
 		werr = ntstatus_to_werror(result);
 		goto done;
 	}
+	if (names.count != num_rids) {
+		werr = WERR_BAD_NET_RESP;
+		goto done;
+	}
+	if (types.count != num_rids) {
+		werr = WERR_BAD_NET_RESP;
+		goto done;
+	}
 
 	for (i=0; i < names.count; i++) {
 		status = add_LOCALGROUP_USERS_INFO_X_buffer(ctx,
diff --git a/source3/rpcclient/cmd_samr.c b/source3/rpcclient/cmd_samr.c
index 0a6dede65c2..f252acce4ff 100644
--- a/source3/rpcclient/cmd_samr.c
+++ b/source3/rpcclient/cmd_samr.c
@@ -2220,6 +2220,14 @@ static NTSTATUS cmd_samr_lookup_rids(struct rpc_pipe_client *cli,
 		goto done;
 
 	/* Display results */
+	if (num_rids != names.count) {
+		status = NT_STATUS_INVALID_NETWORK_RESPONSE;
+		goto done;
+	}
+	if (num_rids != types.count) {
+		status = NT_STATUS_INVALID_NETWORK_RESPONSE;
+		goto done;
+	}
 
 	for (i = 0; i < num_rids; i++) {
 		printf("rid 0x%x: %s (%d)\n",
diff --git a/source3/utils/net_rpc.c b/source3/utils/net_rpc.c
index 582f4500a65..fb67060bd19 100644
--- a/source3/utils/net_rpc.c
+++ b/source3/utils/net_rpc.c
@@ -2889,7 +2889,12 @@ static NTSTATUS rpc_list_group_members(struct net_context *c,
 		if (!NT_STATUS_IS_OK(result)) {
 			return result;
 		}
-
+		if (names.count != this_time) {
+			return NT_STATUS_INVALID_NETWORK_RESPONSE;
+		}
+		if (types.count != this_time) {
+			return NT_STATUS_INVALID_NETWORK_RESPONSE;
+		}
 		/* We only have users as members, but make the output
 		   the same as the output of alias members */
 
diff --git a/source3/winbindd/winbindd_msrpc.c b/source3/winbindd/winbindd_msrpc.c
index b426884e896..17c08c5e2c3 100644
--- a/source3/winbindd/winbindd_msrpc.c
+++ b/source3/winbindd/winbindd_msrpc.c
@@ -744,14 +744,20 @@ static NTSTATUS msrpc_lookup_groupmem(struct winbindd_domain *domain,
 		/* Copy result into array.  The talloc system will take
 		   care of freeing the temporary arrays later on. */
 
-		if (tmp_names.count != tmp_types.count) {
-			return NT_STATUS_UNSUCCESSFUL;
+		if (tmp_names.count != num_lookup_rids) {
+			return NT_STATUS_INVALID_NETWORK_RESPONSE;
+		}
+		if (tmp_types.count != num_lookup_rids) {
+			return NT_STATUS_INVALID_NETWORK_RESPONSE;
 		}
 
 		for (r=0; r<tmp_names.count; r++) {
 			if (tmp_types.ids[r] == SID_NAME_UNKNOWN) {
 				continue;
 			}
+			if (total_names >= *num_names) {
+				break;
+			}
 			(*names)[total_names] = fill_domain_username_talloc(
 				mem_ctx, domain->name,
 				tmp_names.names[r].string, true);
diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c
index 344f2dc6280..c560a6b794d 100644
--- a/source3/winbindd/winbindd_rpc.c
+++ b/source3/winbindd/winbindd_rpc.c
@@ -871,14 +871,20 @@ NTSTATUS rpc_lookup_groupmem(TALLOC_CTX *mem_ctx,
 
 	/* Copy result into array.  The talloc system will take
 	   care of freeing the temporary arrays later on. */
-	if (tmp_names.count != tmp_types.count) {
-		return NT_STATUS_UNSUCCESSFUL;
+	if (tmp_names.count != num_names) {
+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
+	}
+	if (tmp_types.count != num_names) {
+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
 	}
 
 	for (r = 0; r < tmp_names.count; r++) {
 		if (tmp_types.ids[r] == SID_NAME_UNKNOWN) {
 			continue;
 		}
+		if (total_names >= num_names) {
+			break;
+		}
 		names[total_names] = fill_domain_username_talloc(names,
 								 domain_name,
 								 tmp_names.names[r].string,