diff options
author | Joseph Sutton <josephsutton@catalyst.net.nz> | 2023-03-03 17:35:55 +1300 |
---|---|---|
committer | Jule Anger <janger@samba.org> | 2023-03-20 10:03:45 +0100 |
commit | e08188bb9847b4c34d62f7c812d58b07105f5756 (patch) | |
tree | 201bf5965e943d895d53be2c14e1d3f0147da4a3 | |
parent | b98f8c1af7770b49a447fdcc67ea64d98454955f (diff) | |
download | samba-e08188bb9847b4c34d62f7c812d58b07105f5756.tar.gz |
CVE-2023-0614 ldb: Filter on search base before redacting message
Redaction may be expensive if we end up needing to fetch a security
descriptor to verify rights to an attribute. Checking the search scope
is probably cheaper, so do that first.
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-- | lib/ldb/common/ldb_match.c | 8 | ||||
-rw-r--r-- | lib/ldb/include/ldb_private.h | 8 | ||||
-rw-r--r-- | lib/ldb/ldb_key_value/ldb_kv_index.c | 40 | ||||
-rw-r--r-- | lib/ldb/ldb_key_value/ldb_kv_search.c | 14 |
4 files changed, 47 insertions, 23 deletions
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c index 267498560e6..463a24ce3bc 100644 --- a/lib/ldb/common/ldb_match.c +++ b/lib/ldb/common/ldb_match.c @@ -39,10 +39,10 @@ /* check if the scope matches in a search result */ -static int ldb_match_scope(struct ldb_context *ldb, - struct ldb_dn *base, - struct ldb_dn *dn, - enum ldb_scope scope) +int ldb_match_scope(struct ldb_context *ldb, + struct ldb_dn *base, + struct ldb_dn *dn, + enum ldb_scope scope) { int ret = 0; diff --git a/lib/ldb/include/ldb_private.h b/lib/ldb/include/ldb_private.h index b0a42f6421c..5e29de34f79 100644 --- a/lib/ldb/include/ldb_private.h +++ b/lib/ldb/include/ldb_private.h @@ -322,6 +322,14 @@ int ldb_match_message(struct ldb_context *ldb, const struct ldb_parse_tree *tree, enum ldb_scope scope, bool *matched); +/* + check if the scope matches in a search result +*/ +int ldb_match_scope(struct ldb_context *ldb, + struct ldb_dn *base, + struct ldb_dn *dn, + enum ldb_scope scope); + /* Reallocate elements to drop any excess capacity. */ void ldb_msg_shrink_to_fit(struct ldb_message *msg); diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c index 163052fecf7..aac0913f431 100644 --- a/lib/ldb/ldb_key_value/ldb_kv_index.c +++ b/lib/ldb/ldb_key_value/ldb_kv_index.c @@ -2428,31 +2428,37 @@ static int ldb_kv_index_filter(struct ldb_kv_private *ldb_kv, return LDB_ERR_OPERATIONS_ERROR; } - if (ldb->redact.callback != NULL) { - ret = ldb->redact.callback(ldb->redact.module, ac->req, msg); - if (ret != LDB_SUCCESS) { - talloc_free(msg); - return ret; - } - } - /* * We trust the index for LDB_SCOPE_ONELEVEL * unless the index key has been truncated. * * LDB_SCOPE_BASE is not passed in by our only caller. */ - if (ac->scope == LDB_SCOPE_ONELEVEL && - ldb_kv->cache->one_level_indexes && - scope_one_truncation == KEY_NOT_TRUNCATED) { - ret = ldb_match_message(ldb, msg, ac->tree, - ac->scope, &matched); - } else { - ret = ldb_match_msg_error(ldb, msg, - ac->tree, ac->base, - ac->scope, &matched); + if (ac->scope != LDB_SCOPE_ONELEVEL || + !ldb_kv->cache->one_level_indexes || + scope_one_truncation != KEY_NOT_TRUNCATED) + { + /* + * The redaction callback may be expensive to call if it + * fetches a security descriptor. Check the DN early and + * bail out if it doesn't match the base. + */ + if (!ldb_match_scope(ldb, ac->base, msg->dn, ac->scope)) { + talloc_free(msg); + continue; + } + } + + if (ldb->redact.callback != NULL) { + ret = ldb->redact.callback(ldb->redact.module, ac->req, msg); + if (ret != LDB_SUCCESS) { + talloc_free(msg); + return ret; + } } + ret = ldb_match_message(ldb, msg, ac->tree, + ac->scope, &matched); if (ret != LDB_SUCCESS) { talloc_free(keys); talloc_free(msg); diff --git a/lib/ldb/ldb_key_value/ldb_kv_search.c b/lib/ldb/ldb_key_value/ldb_kv_search.c index d187ba223e1..27f68caef01 100644 --- a/lib/ldb/ldb_key_value/ldb_kv_search.c +++ b/lib/ldb/ldb_key_value/ldb_kv_search.c @@ -395,6 +395,16 @@ static int search_func(_UNUSED_ struct ldb_kv_private *ldb_kv, } } + /* + * The redaction callback may be expensive to call if it fetches a + * security descriptor. Check the DN early and bail out if it doesn't + * match the base. + */ + if (!ldb_match_scope(ldb, ac->base, msg->dn, ac->scope)) { + talloc_free(msg); + return 0; + } + if (ldb->redact.callback != NULL) { ret = ldb->redact.callback(ldb->redact.module, ac->req, msg); if (ret != LDB_SUCCESS) { @@ -404,8 +414,8 @@ static int search_func(_UNUSED_ struct ldb_kv_private *ldb_kv, } /* see if it matches the given expression */ - ret = ldb_match_msg_error(ldb, msg, - ac->tree, ac->base, ac->scope, &matched); + ret = ldb_match_message(ldb, msg, + ac->tree, ac->scope, &matched); if (ret != LDB_SUCCESS) { talloc_free(msg); ac->error = LDB_ERR_OPERATIONS_ERROR; |