CVE-2013-4408:s3:Ensure LookupSids replies arrays are range checked.

[samba.git] / source3 / winbindd / wb_lookupsids.c

diff --git a/source3/winbindd/wb_lookupsids.c b/source3/winbindd/wb_lookupsids.c
index 8f431ab4fc6d5992ac3e6a72a709dabeee13fb81..e10d511493860688c6641dc0b5e5145f126b5344 100644 (file)
--- a/source3/winbindd/wb_lookupsids.c
+++ b/source3/winbindd/wb_lookupsids.c
@@ -123,11 +123,6 @@ struct tevent_req *wb_lookupsids_send(TALLOC_CTX *mem_ctx,
        state->sids = sids;
        state->num_sids = num_sids;
 
-       if (num_sids == 0) {
-               tevent_req_done(req);
-               return tevent_req_post(req, ev);
-       }
-
        state->single_sids = talloc_array(state, uint32_t, num_sids);
        if (tevent_req_nomem(state->single_sids, req)) {
                return tevent_req_post(req, ev);
@@ -153,6 +148,11 @@ struct tevent_req *wb_lookupsids_send(TALLOC_CTX *mem_ctx,
                return tevent_req_post(req, ev);
        }
 
+       if (num_sids == 0) {
+               tevent_req_done(req);
+               return tevent_req_post(req, ev);
+       }
+
        for (i=0; i<num_sids; i++) {
                struct wb_lookupsids_domain *d;
 
@@ -185,7 +185,7 @@ static bool wb_lookupsids_next(struct tevent_req *req,
 
                d = &state->domains[state->domains_done];
 
-               if (sid_check_is_domain(&d->sid)) {
+               if (sid_check_is_our_sam(&d->sid)) {
                        state->rids.num_rids = d->sids.num_sids;
                        state->rids.rids = talloc_array(state, uint32_t,
                                                        state->rids.num_rids);
@@ -198,7 +198,7 @@ static bool wb_lookupsids_next(struct tevent_req *req,
                        }
                        subreq = dcerpc_wbint_LookupRids_send(
                                state, state->ev, dom_child_handle(d->domain),
-                               &state->rids, &state->domain_name,
+                               &d->sid, &state->rids, &state->domain_name,
                                &state->rid_names);
                        if (tevent_req_nomem(subreq, req)) {
                                return false;
@@ -255,7 +255,7 @@ static bool wb_lookupsids_bulk(const struct dom_sid *sid)
                return false;
        }
 
-       if (sid_check_is_in_our_domain(sid)) {
+       if (sid_check_is_in_our_sam(sid)) {
                /*
                 * Passdb lookup via lookuprids
                 */
@@ -367,7 +367,7 @@ static bool wb_lookupsids_find_dom_idx(struct lsa_DomainInfo *domain,
        struct lsa_DomainInfo *new_domain;
 
        for (i=0; i<list->count; i++) {
-               if (sid_equal(domain->sid, list->domains[i].sid)) {
+               if (dom_sid_equal(domain->sid, list->domains[i].sid)) {
                        *idx = i;
                        return true;
                }
@@ -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(
@@ -481,7 +484,7 @@ static void wb_lookupsids_done(struct tevent_req *subreq)
                            &state->tmp_domains, &state->tmp_names.names[i],
                            state->res_domains, state->res_names,
                            res_sid_index)) {
-                       tevent_req_nomem(NULL, req);
+                       tevent_req_oom(req);
                        return;
                }
        }
@@ -544,7 +547,7 @@ static void wb_lookupsids_single_done(struct tevent_req *subreq)
                    &src_domains, &src_name,
                    state->res_domains, state->res_names,
                    res_sid_index)) {
-               tevent_req_nomem(NULL, req);
+               tevent_req_oom(req);
                return;
        }
        state->single_sids_done += 1;
@@ -619,7 +622,7 @@ static void wb_lookupsids_lookuprids_done(struct tevent_req *subreq)
                            &src_domains, &src_name,
                            state->res_domains, state->res_names,
                            res_sid_index)) {
-                       tevent_req_nomem(NULL, req);
+                       tevent_req_oom(req);
                        return;
                }
        }
@@ -639,6 +642,28 @@ NTSTATUS wb_lookupsids_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
        if (tevent_req_is_nterror(req, &status)) {
                return status;
        }
+
+       /*
+        * The returned names need to match the given sids,
+        * if not we have a bug in the code!
+        *
+        */
+       if (state->res_names->count != state->num_sids) {
+               DEBUG(0, ("res_names->count = %d, expected %d\n",
+                         state->res_names->count, state->num_sids));
+               return NT_STATUS_INTERNAL_ERROR;
+       }
+
+       /*
+        * Not strictly needed, but it might make debugging in the callers
+        * easier in future, if the talloc_array_length() returns the
+        * expected result...
+        */
+       state->res_domains->domains = talloc_realloc(state->res_domains,
+                                                    state->res_domains->domains,
+                                                    struct lsa_DomainInfo,
+                                                    state->res_domains->count);
+
        *domains = talloc_move(mem_ctx, &state->res_domains);
        *names = talloc_move(mem_ctx, &state->res_names);
        return NT_STATUS_OK;