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>

diff --git a/nsswitch/libwbclient/wbc_sid.c b/nsswitch/libwbclient/wbc_sid.c
index 6df8a3c375f328e5b190a0bbc9544b70f8e72437..35319c536e7ea4faef634803991d8d326ef28286 100644 (file)
--- 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 9d25f59b8c0659573f18f65a9093f4c41247b971..8b822d7c70877b943b7e162ad13b56256cbb540e 100644 (file)
--- 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 330774d9d3d0aa7e727397d889c0f404d5f0cc34..e7e8236012f7d7f43d439fb8d88f70c2e1eaa99e 100644 (file)
--- 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 3aaef5cac21879e4c0bc86508f34923bfcaf35db..9893a7c6d308b8e648ce40fc5d0012b360d18779 100644 (file)
--- 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 2fd735d9dd39b3e44f86fb01bcb08bc7497f515d..32f646cbf265bdc692f74ae01b10bb0348d4962d 100644 (file)
--- 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 9a95e57d8ab09394d1cf3364932d257f65e6b87f..ad4ce443a5294343bb68ec15dcbebab119e5eb48 100644 (file)
--- 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 4cfdf937259d141c2c5c9c3e81a3bb6af8fe5774..d705abdca49593bff55333e1bb7f92cc7ade9bfa 100644 (file)
--- 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 5d530ca149ba8631da7e29edebd770af361e8d5c..01ff4a42e99cd9b80919d8eda5dfec0a158647da 100644 (file)
--- 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;
                }