mirror of
https://github.com/samba-team/samba.git
synced 2025-01-11 05:18:09 +03:00
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>
This commit is contained in:
parent
fffea59001
commit
f154fad3c1
@ -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;
|
||||
|
Loading…
Reference in New Issue
Block a user