[PATCH] Avoid duplicate ids in wbinfo -r
Volker Lendecke
vl at samba.org
Thu Jan 19 08:51:08 UTC 2017
Hi!
Review appreciated!
Thanks, Volker
-------------- next part --------------
>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