summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJoseph Sutton <josephsutton@catalyst.net.nz>2023-03-03 17:35:55 +1300
committerJule Anger <janger@samba.org>2023-03-20 10:03:38 +0100
commita74571b49f5476cde430f11cd7bc256f17925fe8 (patch)
treebe6dbff4ac094640340b9f30e4cb7c4e8ce88aac
parentd60683e5e9daf243e9a2acc203b567c3a6c92567 (diff)
downloadsamba-a74571b49f5476cde430f11cd7bc256f17925fe8.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.c8
-rw-r--r--lib/ldb/include/ldb_private.h8
-rw-r--r--lib/ldb/ldb_key_value/ldb_kv_index.c40
-rw-r--r--lib/ldb/ldb_key_value/ldb_kv_search.c14
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;