143 lines
5.3 KiB
Diff
143 lines
5.3 KiB
Diff
|
|
From 9e07df4e268fd49358382e8d90c2d151efb66569 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Joseph Sutton <josephsutton@catalyst.net.nz>
|
||
|
|
Date: Tue, 7 Feb 2023 09:35:24 +1300
|
||
|
|
Subject: [PATCH 15/34] 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>
|
||
|
|
|
||
|
|
Conflict: NA
|
||
|
|
Reference: https://attachments.samba.org/attachment.cgi?id=17821
|
||
|
|
---
|
||
|
|
.../dsdb/samdb/ldb_modules/extended_dn_in.c | 40 +++++++++++++++----
|
||
|
|
1 file 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;
|
||
|
|
--
|
||
|
|
2.25.1
|