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>
Autobuild-User(master): Karolin Seeger <kseeger@samba.org>
Autobuild-Date(master): Mon Dec  9 09:00:41 CET 2013 on sn-devel-104

diff --git a/source3/lib/netapi/group.c b/source3/lib/netapi/group.c
index ea0414614d4d17d3b0f3fd5e64deab3e11baa22d..6d9b248968a539585de04d8d1d625bb084b9a477 100644 (file)
--- 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++) {
@@ -1624,6 +1632,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 72d4a8bdc396baaf3d75745def394ed9636c98bf..4a39f6976d0870198ff00759a542ef5122daf949 100644 (file)
--- a/source3/lib/netapi/user.c
+++ b/source3/lib/netapi/user.c
@@ -3114,6 +3114,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 5bc8c0b57b85e0f6206983614fa2a58ace6cdb45..87882c3ce4e9bf9937ef21fd806090fd3c39ba18 100644 (file)
--- a/source3/rpcclient/cmd_samr.c
+++ b/source3/rpcclient/cmd_samr.c
@@ -2223,6 +2223,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 a11ad3df9287a58c2a4fe3ea6a01ee1155a776b5..816705e86e75f4bbf32a2f74db717e6c0a909f88 100644 (file)
--- a/source3/utils/net_rpc.c
+++ b/source3/utils/net_rpc.c
@@ -3071,7 +3071,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 61447d3acf016bf059a92c29aa2ee66d7aff18c5..426d64cf1f930166f3cac5d85e596888d29c8abb 100644 (file)
--- 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 6b88c8413c5dc4d19b568c022e585809b0c0f073..0986d825d9aa4fd19b7bbb50fb270fdecd97407a 100644 (file)
--- 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,