From ae3d2737949d9702c5526490c2155740a96a9adb Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 13 Mar 2023 14:25:56 +1300 Subject: 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 Reviewed-by: Joseph Sutton (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] --- lib/ldb/common/ldb_match.c | 60 ++++++++++++++++++++++++++++++++++++++-------- 1 file 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; } -- cgit v1.2.1