From faf6b99248fc060fb3813761b1b2ea4b463bb4f9 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 10 Jan 2017 13:24:22 +0000 Subject: [PATCH 1/9] winbind: Fix CID 1398533 Resource leak Not really a leak due to talloc, but this way it's clear Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison --- source3/winbindd/winbindd_msrpc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/winbindd/winbindd_msrpc.c b/source3/winbindd/winbindd_msrpc.c index 4b742c4c58b..0d0e4ca6506 100644 --- a/source3/winbindd/winbindd_msrpc.c +++ b/source3/winbindd/winbindd_msrpc.c @@ -90,6 +90,7 @@ static NTSTATUS msrpc_query_user_list(struct winbindd_domain *domain, } done: + TALLOC_FREE(rids); TALLOC_FREE(tmp_ctx); return status; } -- 2.11.0 From c544dd9a5b2525ff2e0d01d5671d45c370f520fd Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 10 Jan 2017 13:26:13 +0000 Subject: [PATCH 2/9] winbind: Fix CID 1398533 Resource leak Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison --- source3/winbindd/winbindd_rpc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c index bb8af45c896..fcc636620a4 100644 --- a/source3/winbindd/winbindd_rpc.c +++ b/source3/winbindd/winbindd_rpc.c @@ -77,6 +77,7 @@ NTSTATUS rpc_query_user_list(TALLOC_CTX *mem_ctx, } if (!NT_STATUS_IS_OK(result)) { if (!NT_STATUS_EQUAL(result, STATUS_MORE_ENTRIES)) { + TALLOC_FREE(rids); return result; } } -- 2.11.0 From 43c2c848c9af29065dd6f1386c621363e8dc2789 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 10 Jan 2017 13:24:22 +0000 Subject: [PATCH 3/9] winbind: Fix CID 1398531 Resource leak Not really a leak due to talloc, but this way it's clear Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison --- source3/winbindd/winbindd_samr.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c index dd674965f17..224f1058348 100644 --- a/source3/winbindd/winbindd_samr.c +++ b/source3/winbindd/winbindd_samr.c @@ -208,6 +208,7 @@ done: dcerpc_samr_Close(b, mem_ctx, &dom_pol, &result); } + TALLOC_FREE(rids); TALLOC_FREE(tmp_ctx); return status; } -- 2.11.0 From e9c117a89022b69a2f30337137964ab21fd3ac27 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 10 Jan 2017 13:28:41 +0000 Subject: [PATCH 4/9] winbind: Fix CID 1398530 Resource leak Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison --- source3/winbindd/winbindd_ads.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c index 83579686ff4..b14f21e3644 100644 --- a/source3/winbindd/winbindd_ads.c +++ b/source3/winbindd/winbindd_ads.c @@ -356,8 +356,9 @@ static NTSTATUS query_user_list(struct winbindd_domain *domain, } if (!ads_pull_sid(ads, msg, "objectSid", &user_sid)) { - DBG_INFO("No sid for %s !?\n", - ads_get_dn(ads, talloc_tos(), msg)); + char *dn = ads_get_dn(ads, talloc_tos(), msg); + DBG_INFO("No sid for %s !?\n", dn); + TALLOC_FREE(dn); continue; } -- 2.11.0 From 9065b1c3fd01af6573a3da0a57f70ebb4f535ba2 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 10 Jan 2017 13:29:38 +0000 Subject: [PATCH 5/9] winbind: Fix CID 1398530 Resource leak Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Wed Jan 11 04:38:25 CET 2017 on sn-devel-144 --- source3/winbindd/winbindd_cache.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c index d92c050ee70..4431cb52b64 100644 --- a/source3/winbindd/winbindd_cache.c +++ b/source3/winbindd/winbindd_cache.c @@ -1457,6 +1457,7 @@ do_fetch_cache: rids = talloc_array(mem_ctx, uint32_t, num_rids); if (rids == NULL) { + centry_free(centry); return NT_STATUS_NO_MEMORY; } -- 2.11.0 From 816d307e660d20fabc8bfe290da1bbd6c4b63d78 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 11 Jan 2017 11:52:44 -0800 Subject: [PATCH 6/9] winbind: Fix CID 1398534 Dereference before null check Make all query_user_list backends consistent. Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider Autobuild-User(master): Andreas Schneider Autobuild-Date(master): Fri Jan 13 13:33:37 CET 2017 on sn-devel-144 --- source3/winbindd/winbindd_ads.c | 8 ++++---- source3/winbindd/winbindd_samr.c | 6 ++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c index b14f21e3644..077c6ec7b7c 100644 --- a/source3/winbindd/winbindd_ads.c +++ b/source3/winbindd/winbindd_ads.c @@ -293,14 +293,12 @@ static NTSTATUS query_user_list(struct winbindd_domain *domain, ADS_STRUCT *ads = NULL; const char *attrs[] = { "sAMAccountType", "objectSid", NULL }; int count; - uint32_t *rids; + uint32_t *rids = NULL; ADS_STATUS rc; LDAPMessage *res = NULL; LDAPMessage *msg = NULL; NTSTATUS status = NT_STATUS_UNSUCCESSFUL; - *prids = NULL; - DEBUG(3,("ads: query_user_list\n")); if ( !winbindd_can_contact_domain( domain ) ) { @@ -375,7 +373,9 @@ static NTSTATUS query_user_list(struct winbindd_domain *domain, } rids = talloc_realloc(mem_ctx, rids, uint32_t, count); - *prids = rids; + if (prids != NULL) { + *prids = rids; + } status = NT_STATUS_OK; diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c index 224f1058348..1a73fc4fcc6 100644 --- a/source3/winbindd/winbindd_samr.c +++ b/source3/winbindd/winbindd_samr.c @@ -169,15 +169,13 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain, { struct rpc_pipe_client *samr_pipe = NULL; struct policy_handle dom_pol = { 0 }; - uint32_t *rids; + uint32_t *rids = NULL; TALLOC_CTX *tmp_ctx; NTSTATUS status, result; struct dcerpc_binding_handle *b = NULL; DEBUG(3,("samr_query_user_list\n")); - *prids = NULL; - tmp_ctx = talloc_stackframe(); if (tmp_ctx == NULL) { return NT_STATUS_NO_MEMORY; @@ -199,7 +197,7 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain, goto done; } - if (prids) { + if (prids != NULL) { *prids = talloc_move(mem_ctx, &rids); } -- 2.11.0 From 0450fe73ae6acf399997571d3631744a283277de Mon Sep 17 00:00:00 2001 From: Aurelien Aptel Date: Tue, 17 Jan 2017 14:39:03 +0100 Subject: [PATCH 7/9] s3/winbindd: fix invalid free coverity fix. TALLOC_FREE() might be called on uninitialized 'rids' at the end of the function in case of an early error. Initialize it to NULL to turn the TALLOC_FREE() to a noop in this case. Signed-off-by: Aurelien Aptel Reviewed-by: Volker Lendecke Reviewed-by: David Disseldorp Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Wed Jan 18 17:19:39 CET 2017 on sn-devel-144 --- source3/winbindd/winbindd_msrpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/winbindd/winbindd_msrpc.c b/source3/winbindd/winbindd_msrpc.c index 0d0e4ca6506..5ace4d1c0bd 100644 --- a/source3/winbindd/winbindd_msrpc.c +++ b/source3/winbindd/winbindd_msrpc.c @@ -53,7 +53,7 @@ static NTSTATUS msrpc_query_user_list(struct winbindd_domain *domain, { struct rpc_pipe_client *samr_pipe = NULL; struct policy_handle dom_pol; - uint32_t *rids; + uint32_t *rids = NULL; TALLOC_CTX *tmp_ctx; NTSTATUS status; -- 2.11.0 From 41398725a6ee2f629c9a6908e6d73f64ac444c58 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Fri, 13 Jan 2017 07:33:24 +0100 Subject: [PATCH 8/9] winbind: Fix a typo Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison --- source3/winbindd/wb_sids2xids.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/winbindd/wb_sids2xids.c b/source3/winbindd/wb_sids2xids.c index 25260be3384..9bb8fa87cf8 100644 --- a/source3/winbindd/wb_sids2xids.c +++ b/source3/winbindd/wb_sids2xids.c @@ -40,7 +40,7 @@ struct wb_sids2xids_state { /* * Domain array to use for the idmap call. The output from * lookupsids cannot be used directly since for migrated - * objects the returned domain SID can be different that the + * objects the returned domain SID can be different than the * original one. The new domain SID cannot be combined with * the RID from the previous domain. * -- 2.11.0 From c23f0eeaaab4e36ff7c9ee03319f6ff4699efe0a Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 18 Jan 2017 16:54:03 +0100 Subject: [PATCH 9/9] winbind: Don't add duplicate IDs in wbinfo -r We look at the netsamlogon_cache entry twice: Once in queryuser and once in lookupusergroups_cached. This can add the group SID twice. Use add_sid_to_array_unique to avoid this. Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Tue Jan 24 02:36:19 CET 2017 on sn-devel-144 --- source3/winbindd/wb_gettoken.c | 81 ++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 43 deletions(-) diff --git a/source3/winbindd/wb_gettoken.c b/source3/winbindd/wb_gettoken.c index d8867c36b9d..07c7fc79a31 100644 --- a/source3/winbindd/wb_gettoken.c +++ b/source3/winbindd/wb_gettoken.c @@ -27,14 +27,15 @@ struct wb_gettoken_state { struct tevent_context *ev; struct dom_sid usersid; bool expand_local_aliases; - int num_sids; + uint32_t num_sids; struct dom_sid *sids; }; -static bool wb_add_rids_to_sids(TALLOC_CTX *mem_ctx, - int *pnum_sids, struct dom_sid **psids, - const struct dom_sid *domain_sid, - int num_rids, uint32_t *rids); +static NTSTATUS wb_add_rids_to_sids(TALLOC_CTX *mem_ctx, + uint32_t *pnum_sids, + struct dom_sid **psids, + const struct dom_sid *domain_sid, + int num_rids, uint32_t *rids); static void wb_gettoken_gotuser(struct tevent_req *subreq); static void wb_gettoken_gotlocalgroups(struct tevent_req *subreq); @@ -70,10 +71,9 @@ static void wb_gettoken_gotuser(struct tevent_req *subreq) subreq, struct tevent_req); struct wb_gettoken_state *state = tevent_req_data( req, struct wb_gettoken_state); - struct dom_sid *sids; struct winbindd_domain *domain; struct wbint_userinfo *info; - uint32_t num_groups; + uint32_t i, num_groups; struct dom_sid *groups; NTSTATUS status; @@ -83,11 +83,10 @@ static void wb_gettoken_gotuser(struct tevent_req *subreq) return; } - sids = talloc_array(state, struct dom_sid, 2); - if (tevent_req_nomem(sids, req)) { + state->sids = talloc_array(state, struct dom_sid, 2); + if (tevent_req_nomem(state->sids, req)) { return; } - state->sids = sids; state->num_sids = 2; sid_copy(&state->sids[0], &info->user_sid); @@ -102,21 +101,14 @@ static void wb_gettoken_gotuser(struct tevent_req *subreq) return; } - if (num_groups + state->num_sids < num_groups) { - tevent_req_nterror(req, NT_STATUS_INTEGER_OVERFLOW); - return; - } + for (i=0; isids, &state->num_sids); - sids = talloc_realloc(state, state->sids, struct dom_sid, - state->num_sids+num_groups); - if (tevent_req_nomem(sids, req)) { - return; + if (tevent_req_nterror(req, status)) { + return; + } } - state->sids = sids; - - memcpy(&state->sids[state->num_sids], groups, - num_groups * sizeof(struct dom_sid)); - state->num_sids += num_groups; if (!state->expand_local_aliases) { tevent_req_done(req); @@ -156,9 +148,10 @@ static void wb_gettoken_gotlocalgroups(struct tevent_req *subreq) if (tevent_req_nterror(req, status)) { return; } - if (!wb_add_rids_to_sids(state, &state->num_sids, &state->sids, - get_global_sam_sid(), num_rids, rids)) { - tevent_req_nterror(req, NT_STATUS_NO_MEMORY); + + status = wb_add_rids_to_sids(state, &state->num_sids, &state->sids, + get_global_sam_sid(), num_rids, rids); + if (tevent_req_nterror(req, status)) { return; } TALLOC_FREE(rids); @@ -196,9 +189,9 @@ static void wb_gettoken_gotbuiltins(struct tevent_req *subreq) if (tevent_req_nterror(req, status)) { return; } - if (!wb_add_rids_to_sids(state, &state->num_sids, &state->sids, - &global_sid_Builtin, num_rids, rids)) { - tevent_req_nterror(req, NT_STATUS_NO_MEMORY); + status = wb_add_rids_to_sids(state, &state->num_sids, &state->sids, + &global_sid_Builtin, num_rids, rids); + if (tevent_req_nterror(req, status)) { return; } tevent_req_done(req); @@ -219,24 +212,26 @@ NTSTATUS wb_gettoken_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, return NT_STATUS_OK; } -static bool wb_add_rids_to_sids(TALLOC_CTX *mem_ctx, - int *pnum_sids, struct dom_sid **psids, - const struct dom_sid *domain_sid, - int num_rids, uint32_t *rids) +static NTSTATUS wb_add_rids_to_sids(TALLOC_CTX *mem_ctx, + uint32_t *pnum_sids, + struct dom_sid **psids, + const struct dom_sid *domain_sid, + int num_rids, uint32_t *rids) { - struct dom_sid *sids; int i; - sids = talloc_realloc(mem_ctx, *psids, struct dom_sid, - *pnum_sids + num_rids); - if (sids == NULL) { - return false; - } for (i=0; i