[PATCH] Avoid duplicate ids in wbinfo -r
Jeremy Allison
jra at samba.org
Mon Jan 23 21:44:49 UTC 2017
On Thu, Jan 19, 2017 at 09:51:08AM +0100, Volker Lendecke wrote:
> Hi!
>
> Review appreciated!
LGTM. RB+ - pushed !
> From dae8c03255b6af40a7df1d6f36d9fd7214576558 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Fri, 13 Jan 2017 07:33:24 +0100
> Subject: [PATCH 1/4] winbind: Fix a typo
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> 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 25260be..9bb8fa8 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.1.4
>
>
> From 1379b9919bbbb784209734b78d20f9a28a20bf82 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 18 Jan 2017 16:43:35 +0100
> Subject: [PATCH 2/4] libcli: Do not overwrite pointer on realloc failure
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> libcli/security/util_sid.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/libcli/security/util_sid.c b/libcli/security/util_sid.c
> index 2f3fceb..2ab47f2 100644
> --- a/libcli/security/util_sid.c
> +++ b/libcli/security/util_sid.c
> @@ -337,12 +337,14 @@ int sid_compare_domain(const struct dom_sid *sid1, const struct dom_sid *sid2)
> NTSTATUS add_sid_to_array(TALLOC_CTX *mem_ctx, const struct dom_sid *sid,
> struct dom_sid **sids, uint32_t *num)
> {
> - *sids = talloc_realloc(mem_ctx, *sids, struct dom_sid,
> - (*num)+1);
> - if (*sids == NULL) {
> + struct dom_sid *tmp;
> +
> + tmp = talloc_realloc(mem_ctx, *sids, struct dom_sid, (*num)+1);
> + if (tmp == NULL) {
> *num = 0;
> return NT_STATUS_NO_MEMORY;
> }
> + *sids = tmp;
>
> sid_copy(&((*sids)[*num]), sid);
> *num += 1;
> --
> 2.1.4
>
>
> From a43c5ea40cae49a2560f8e5cd7d64a1d52b5b689 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 18 Jan 2017 16:43:56 +0100
> Subject: [PATCH 3/4] libcli: Add an overflow check
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> libcli/security/util_sid.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/libcli/security/util_sid.c b/libcli/security/util_sid.c
> index 2ab47f2..ac44876 100644
> --- a/libcli/security/util_sid.c
> +++ b/libcli/security/util_sid.c
> @@ -339,6 +339,10 @@ NTSTATUS add_sid_to_array(TALLOC_CTX *mem_ctx, const struct dom_sid *sid,
> {
> struct dom_sid *tmp;
>
> + if ((*num) == UINT32_MAX) {
> + return NT_STATUS_INTEGER_OVERFLOW;
> + }
> +
> tmp = talloc_realloc(mem_ctx, *sids, struct dom_sid, (*num)+1);
> if (tmp == NULL) {
> *num = 0;
> --
> 2.1.4
>
>
> From 3ca1a5eb16722f79b0f77eeb4bd0af94080db2c8 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 18 Jan 2017 16:54:03 +0100
> Subject: [PATCH 4/4] 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 <vl at samba.org>
> ---
> 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 d8867c3..07c7fc7 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; i<num_groups; i++) {
> + status = add_sid_to_array_unique(
> + state, &groups[i], &state->sids, &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<num_rids; i++) {
> - sid_compose(&sids[i+*pnum_sids], domain_sid, rids[i]);
> + NTSTATUS status;
> + struct dom_sid sid;
> +
> + sid_compose(&sid, domain_sid, rids[i]);
> +
> + status = add_sid_to_array_unique(
> + mem_ctx, &sid, psids, pnum_sids);
> + if (!NT_STATUS_IS_OK(status)) {
> + return status;
> + }
> }
>
> - *pnum_sids += num_rids;
> - *psids = sids;
> - return true;
> + return NT_STATUS_OK;
> }
> --
> 2.1.4
>
More information about the samba-technical
mailing list