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>

diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_in.c b/source4/dsdb/samdb/ldb_modules/extended_dn_in.c
index 2d0513a738b631f9e1e43d7b66d4a05707ef460d..1dc1e1f2d4294c171a9dc2f1dab6ea240ad96037 100644 (file)
--- 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;