diff options
author | Andrew Bartlett <abartlet@samba.org> | 2023-03-13 14:25:56 +1300 |
---|---|---|
committer | Jule Anger <janger@samba.org> | 2023-03-20 10:03:50 +0100 |
commit | 680b865f183d3103cd7d465e6b921fb5f28627b8 (patch) | |
tree | af5f7032408dc1b16238cd3ee632ef1d49b64cf3 | |
parent | 6bd15c87430efc103742d87917dcb14226f24169 (diff) | |
download | samba-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.c | 60 |
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; } |