summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndrew Bartlett <abartlet@samba.org>2023-03-13 14:25:56 +1300
committerJule Anger <janger@samba.org>2023-03-20 10:03:37 +0100
commitae3d2737949d9702c5526490c2155740a96a9adb (patch)
treeb10af9ca8873576c2155a798ab2b9a055ee92d03
parentf2461834bbc5266fe48fd32bd3aa14bd721604da (diff)
downloadsamba-ae3d2737949d9702c5526490c2155740a96a9adb.tar.gz
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]
-rw-r--r--lib/ldb/common/ldb_match.c60
1 files changed, 50 insertions, 10 deletions
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 2f4d41f3441..51376871b4c 100644
--- 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;
}