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>

diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 267498560e6b21789caa6e5c0718a2b5814e67b9..463a24ce3bc06d7f5828c6821c5a9468e1a47a82 100644 (file)
--- 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 b0a42f6421c973e947c35bfee66bc500c471cc15..5e29de34f791293032d6ef968a27c5676357cb55 100644 (file)
--- 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 163052fecf7dfd7fe840973c98b800e7dc058c52..aac0913f431a2b29f668326b94eec64d565e5e27 100644 (file)
--- 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 d187ba223e1128a5072c5f310bd7025ed2a3d5cd..27f68caef01bc82cbb13faab9d1182da463400ce 100644 (file)
--- 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;