From a0c888bd0ed2ec5b4d84f9df241bebd5d428818c Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 3 Mar 2023 17:35:55 +1300
Subject: [PATCH 28/34] 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>

Conflict: NA
Reference: https://attachments.samba.org/attachment.cgi?id=17821
---
 lib/ldb/common/ldb_match.c            |  8 +++---
 lib/ldb/include/ldb_private.h         |  8 ++++++
 lib/ldb/ldb_key_value/ldb_kv_index.c  | 40 +++++++++++++++------------
 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 645cb695ef1..8e27ecbe723 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -38,10 +38,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;
-- 
2.25.1