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 471f71b084b0661e33959396936bc8d4ad24556d..0877ed04aeaec48c6d31d2cd0e04240a56af90f3 100644 (file)
--- a/nsswitch/libwbclient/wbc_sid.c
+++ b/nsswitch/libwbclient/wbc_sid.c
@@ -427,6 +427,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 61acd1aa9e1ba6bbc6480a6df19dc50ca1a28604..cc75fc30a168a177700e13ed559fe811ba736a55 100644 (file)
--- a/nsswitch/wbinfo.c
+++ b/nsswitch/wbinfo.c
@@ -1395,15 +1395,25 @@ 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));
 
+               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,
-                                domains[names[i].domain_index].short_name,
+                                domain,
                                 names[i].type);
                } else {
                        d_printf("%s -> %s%c%s %d\n", sidstr,
-                                domains[names[i].domain_index].short_name,
+                                domain,
                                 winbind_separator(),
                                 names[i].name, names[i].type);
                }

diff --git a/source3/rpc_client/cli_lsarpc.c b/source3/rpc_client/cli_lsarpc.c
index 126f3705a3ad7611dbbc028a5eae40a85249f01a..7cadd6edacd909a8a2452b14ec52ae0a8eebe271 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 503e0fba76a56d535dcff6ab96c1593bdaecbb52..0fb371990c4a6fee6a8b5c2ab95754b7936ee959 100644 (file)
--- a/source3/rpcclient/cmd_lsarpc.c
+++ b/source3/rpcclient/cmd_lsarpc.c
@@ -457,7 +457,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;
@@ -513,9 +513,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 2c4ebda3eb7a18f2e7ea76821c20f0430e2c0f04..e10d511493860688c6641dc0b5e5145f126b5344 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 44deeb071c946a8118c7217cec5e4b6957f39596..7345ea798ed91d47a059066a3ad051c00d448e66 100644 (file)
--- a/source3/winbindd/winbindd_rpc.c
+++ b/source3/winbindd/winbindd_rpc.c
@@ -1084,6 +1084,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(mem_ctx, struct lsa_TransNameArray);
        if (names == NULL) {
                return NT_STATUS_NO_MEMORY;
@@ -1099,6 +1103,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;
@@ -1114,6 +1128,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);
@@ -1140,6 +1155,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 812f953ce853f26b7598dd4ee0dd780185c6cb98..cc0dae5984fae681f9e4d0a80af6a7f9d4db217f 100644 (file)
--- a/source4/libcli/util/clilsa.c
+++ b/source4/libcli/util/clilsa.c
@@ -260,7 +260,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 2af8567fd4cfe1358d74238f4baa007a5557b4cb..e3de2eb38ea923c60a4e744e7de9b0d4193da1ba 100644 (file)
--- a/source4/winbind/wb_async_helpers.c
+++ b/source4/winbind/wb_async_helpers.c
@@ -122,6 +122,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;
@@ -142,9 +148,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;
                }