From acab72eaf3fd762f7085a5cec4225cfca8616a63 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 7 Nov 2013 20:38:01 -0800 Subject: [PATCH] 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 Signed-off-by: Jeremy Allison --- nsswitch/libwbclient/wbc_sid.c | 7 +++++++ nsswitch/wbinfo.c | 23 ++++++++++++++++++++--- source3/rpc_client/cli_lsarpc.c | 17 ++++++++++++++++- source3/rpcclient/cmd_lsarpc.c | 7 +++++-- source3/winbindd/wb_lookupsids.c | 3 +++ source3/winbindd/winbindd_rpc.c | 32 ++++++++++++++++++++++++++++++++ source4/libcli/util/clilsa.c | 16 +++++++++++++++- source4/winbind/wb_async_helpers.c | 13 ++++++++++++- 8 files changed, 110 insertions(+), 8 deletions(-) diff --git a/nsswitch/libwbclient/wbc_sid.c b/nsswitch/libwbclient/wbc_sid.c index bab69331087..82ac339865e 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_domains) { + goto wbc_err_invalid; + } + if (*q != ' ') { goto wbc_err_invalid; } diff --git a/nsswitch/wbinfo.c b/nsswitch/wbinfo.c index abe4844b2e1..e3eabca1067 100644 --- a/nsswitch/wbinfo.c +++ b/nsswitch/wbinfo.c @@ -1386,11 +1386,28 @@ static bool wbinfo_lookup_sids(const char *arg) } for (i=0; i %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 = ""; + } else if (names[i].domain_index < 0) { + domain = ""; + } 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); + } } wbcFreeMemory(names); wbcFreeMemory(domains); diff --git a/source3/rpc_client/cli_lsarpc.c b/source3/rpc_client/cli_lsarpc.c index 126f3705a3a..7cadd6edacd 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 cbc089f7391..a7ee4e48902 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 2c4ebda3eb7..e10d5114938 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 44deeb071c9..7345ea798ed 100644 --- 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 812f953ce85..cc0dae5984f 100644 --- 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 2af8567fd4c..e3de2eb38ea 100644 --- 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; } -- 2.11.4.GIT