diff options
author | Joseph Sutton <josephsutton@catalyst.net.nz> | 2023-02-07 09:35:24 +1300 |
---|---|---|
committer | Jule Anger <janger@samba.org> | 2023-03-20 10:03:38 +0100 |
commit | d096cd4ed92bd96523c2dbe42e99fa17783a7395 (patch) | |
tree | 107a1e393441cefad89ac9bf580880c4bc8d9ef3 | |
parent | 4bbdd6709bfe2ba31cee8968751a48a6d454f19e (diff) | |
download | samba-d096cd4ed92bd96523c2dbe42e99fa17783a7395.tar.gz |
CVE-2023-0614 s4:dsdb/extended_dn_in: Don't modify a search tree we don't own
In extended_dn_fix_filter() we had:
req->op.search.tree = ldb_parse_tree_copy_shallow(req, req->op.search.tree);
which overwrote the parse tree on an existing ldb request with a fixed
up tree. This became a problem if a module performed another search with
that same request structure, as extended_dn_in would try to fix up the
already-modified tree for a second time. The fixed-up tree element now
having an extended DN, it would fall foul of the ldb_dn_match_allowed()
check in extended_dn_filter_callback(), and be replaced with an
ALWAYS_FALSE match rule. In practice this meant that <GUID={}> searches
would only work for one search in an ldb request, and fail for
subsequent ones.
Fix this by creating a new request with the modified tree, and leaving
the original request unmodified.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
-rw-r--r-- | source4/dsdb/samdb/ldb_modules/extended_dn_in.c | 40 |
1 files changed, 32 insertions, 8 deletions
diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_in.c b/source4/dsdb/samdb/ldb_modules/extended_dn_in.c index 2d0513a738b..1dc1e1f2d42 100644 --- a/source4/dsdb/samdb/ldb_modules/extended_dn_in.c +++ b/source4/dsdb/samdb/ldb_modules/extended_dn_in.c @@ -48,6 +48,7 @@ struct extended_search_context { struct ldb_module *module; struct ldb_request *req; + struct ldb_parse_tree *tree; struct ldb_dn *basedn; struct ldb_dn *dn; char *wellknown_object; @@ -200,7 +201,7 @@ static int extended_base_callback(struct ldb_request *req, struct ldb_reply *are ldb_module_get_ctx(ac->module), ac->req, ac->basedn, ac->req->op.search.scope, - ac->req->op.search.tree, + ac->tree, ac->req->op.search.attrs, ac->req->controls, ac, extended_final_callback, @@ -515,11 +516,14 @@ static int extended_dn_filter_callback(struct ldb_parse_tree *tree, void *privat */ static int extended_dn_fix_filter(struct ldb_module *module, struct ldb_request *req, - uint32_t default_dsdb_flags) + uint32_t default_dsdb_flags, + struct ldb_parse_tree **down_tree) { struct extended_dn_filter_ctx *filter_ctx; int ret; + *down_tree = NULL; + filter_ctx = talloc_zero(req, struct extended_dn_filter_ctx); if (filter_ctx == NULL) { return ldb_module_oom(module); @@ -550,12 +554,12 @@ static int extended_dn_fix_filter(struct ldb_module *module, filter_ctx->test_only = false; filter_ctx->matched = false; - req->op.search.tree = ldb_parse_tree_copy_shallow(req, req->op.search.tree); - if (req->op.search.tree == NULL) { + *down_tree = ldb_parse_tree_copy_shallow(req, req->op.search.tree); + if (*down_tree == NULL) { return ldb_oom(ldb_module_get_ctx(module)); } - ret = ldb_parse_tree_walk(req->op.search.tree, extended_dn_filter_callback, filter_ctx); + ret = ldb_parse_tree_walk(*down_tree, extended_dn_filter_callback, filter_ctx); if (ret != LDB_SUCCESS) { talloc_free(filter_ctx); return ret; @@ -572,7 +576,8 @@ static int extended_dn_fix_filter(struct ldb_module *module, static int extended_dn_in_fix(struct ldb_module *module, struct ldb_request *req, struct ldb_dn *dn) { struct extended_search_context *ac; - struct ldb_request *down_req; + struct ldb_request *down_req = NULL; + struct ldb_parse_tree *down_tree = NULL; int ret; struct ldb_dn *base_dn = NULL; enum ldb_scope base_dn_scope = LDB_SCOPE_BASE; @@ -595,7 +600,7 @@ static int extended_dn_in_fix(struct ldb_module *module, struct ldb_request *req } if (req->operation == LDB_SEARCH) { - ret = extended_dn_fix_filter(module, req, dsdb_flags); + ret = extended_dn_fix_filter(module, req, dsdb_flags, &down_tree); if (ret != LDB_SUCCESS) { return ret; } @@ -603,7 +608,25 @@ static int extended_dn_in_fix(struct ldb_module *module, struct ldb_request *req if (!ldb_dn_has_extended(dn)) { /* Move along there isn't anything to see here */ - return ldb_next_request(module, req); + if (down_tree == NULL) { + down_req = req; + } else { + ret = ldb_build_search_req_ex(&down_req, + ldb_module_get_ctx(module), req, + req->op.search.base, + req->op.search.scope, + down_tree, + req->op.search.attrs, + req->controls, + req, dsdb_next_callback, + req); + if (ret != LDB_SUCCESS) { + return ret; + } + LDB_REQ_SET_LOCATION(down_req); + } + + return ldb_next_request(module, down_req); } else { /* It looks like we need to map the DN */ const struct ldb_val *sid_val, *guid_val, *wkguid_val; @@ -690,6 +713,7 @@ static int extended_dn_in_fix(struct ldb_module *module, struct ldb_request *req ac->module = module; ac->req = req; + ac->tree = (down_tree != NULL) ? down_tree : req->op.search.tree; ac->dn = dn; ac->basedn = NULL; /* Filled in if the search finds the DN by SID/GUID etc */ ac->wellknown_object = wellknown_object; |