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:50 +0100
commit680b865f183d3103cd7d465e6b921fb5f28627b8 (patch)
treeaf5f7032408dc1b16238cd3ee632ef1d49b64cf3
parent6bd15c87430efc103742d87917dcb14226f24169 (diff)
downloadsamba-680b865f183d3103cd7d465e6b921fb5f28627b8.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;
}