CVE-2023-0614 lib/ldb: Avoid allocation and memcpy() for every wildcard match candidate

The value can be quite large, the allocation will take much
longer than the actual match and is repeated per candidate
record.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15331
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
(cherry picked from commit cad96f59a08192df927fb1df4e9787c7f70991a2)

[abartlet@samba.org Included in the security release as this
 makes the new large_ldap.py timeout test more reliable]

diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 2f4d41f34416f293a76162af6c627a3f1e376070..51376871b4c9bdad2aa296fe39d6000b8f09f80e 100644 (file)
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -34,6 +34,7 @@
 
 #include "ldb_private.h"
 #include "dlinklist.h"
+#include "ldb_handlers.h"
 
 /*
   check if the scope matches in a search result
@@ -259,20 +260,42 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
                return LDB_SUCCESS;
        }
 
-       if (a->syntax->canonicalise_fn(ldb, ldb, &value, &val) != 0) {
-               return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
+       /* No need to just copy this value for a binary match */
+       if (a->syntax->canonicalise_fn != ldb_handler_copy) {
+               if (a->syntax->canonicalise_fn(ldb, ldb, &value, &val) != 0) {
+                       return LDB_ERR_INVALID_ATTRIBUTE_SYNTAX;
+               }
+
+               /*
+                * Only set save_p if we allocate (call
+                * a->syntax->canonicalise_fn()), as we
+                * talloc_free(save_p) below to clean up
+                */
+               save_p = val.data;
+       } else {
+               val = value;
        }
 
-       save_p = val.data;
        cnk.data = NULL;
 
        if ( ! tree->u.substring.start_with_wildcard ) {
+               uint8_t *cnk_to_free = NULL;
 
                chunk = tree->u.substring.chunks[c];
-               if (a->syntax->canonicalise_fn(ldb, ldb, chunk, &cnk) != 0) goto mismatch;
+               /* No need to just copy this value for a binary match */
+               if (a->syntax->canonicalise_fn != ldb_handler_copy) {
+                       if (a->syntax->canonicalise_fn(ldb, ldb, chunk, &cnk) != 0) {
+                               goto mismatch;
+                       }
+
+                       cnk_to_free = cnk.data;
+               } else {
+                       cnk = *chunk;
+               }
 
                /* This deals with wildcard prefix searches on binary attributes (eg objectGUID) */
                if (cnk.length > val.length) {
+                       TALLOC_FREE(cnk_to_free);
                        goto mismatch;
                }
                /*
@@ -280,32 +303,47 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
                 * we can cope with this.
                 */
                if (cnk.length == 0) {
+                       TALLOC_FREE(cnk_to_free);
+                       goto mismatch;
+               }
+
+               if (memcmp((char *)val.data, (char *)cnk.data, cnk.length) != 0) {
+                       TALLOC_FREE(cnk_to_free);
                        goto mismatch;
                }
 
-               if (memcmp((char *)val.data, (char *)cnk.data, cnk.length) != 0) goto mismatch;
                val.length -= cnk.length;
                val.data += cnk.length;
                c++;
-               talloc_free(cnk.data);
+               TALLOC_FREE(cnk_to_free);
                cnk.data = NULL;
        }
 
        while (tree->u.substring.chunks[c]) {
                uint8_t *p;
+               uint8_t *cnk_to_free = NULL;
 
                chunk = tree->u.substring.chunks[c];
-               if(a->syntax->canonicalise_fn(ldb, ldb, chunk, &cnk) != 0) {
-                       goto mismatch;
+               /* No need to just copy this value for a binary match */
+               if (a->syntax->canonicalise_fn != ldb_handler_copy) {
+                       if (a->syntax->canonicalise_fn(ldb, ldb, chunk, &cnk) != 0) {
+                               goto mismatch;
+                       }
+
+                       cnk_to_free = cnk.data;
+               } else {
+                       cnk = *chunk;
                }
                /*
                 * Empty strings are returned as length 0. Ensure
                 * we can cope with this.
                 */
                if (cnk.length == 0) {
+                       TALLOC_FREE(cnk_to_free);
                        goto mismatch;
                }
                if (cnk.length > val.length) {
+                       TALLOC_FREE(cnk_to_free);
                        goto mismatch;
                }
 
@@ -320,6 +358,8 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
                        cmp = memcmp(p,
                                     cnk.data,
                                     cnk.length);
+                       TALLOC_FREE(cnk_to_free);
+
                        if (cmp != 0) {
                                goto mismatch;
                        }
@@ -331,15 +371,16 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
                        p = memmem((const void *)val.data, val.length,
                                   (const void *)cnk.data, cnk.length);
                        if (p == NULL) {
+                               TALLOC_FREE(cnk_to_free);
                                goto mismatch;
                        }
                        /* move val to the end of the match */
                        p += cnk.length;
                        val.length -= (p - val.data);
                        val.data = p;
+                       TALLOC_FREE(cnk_to_free);
                }
                c++;
-               TALLOC_FREE(cnk.data);
        }
 
        talloc_free(save_p);
@@ -349,7 +390,6 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
 mismatch:
        *matched = false;
        talloc_free(save_p);
-       talloc_free(cnk.data);
        return LDB_SUCCESS;
 }