s4:cracknames: make consistent use of ldb_binary_encode_string()

A client provided value can be passed into a filter
without going through ldb_binary_encode_string()

But the value is only appended via a to "name=%s",
so there's no leading "(", which means
in lib/ldb/common/ldb_parse.c:356: the value-parsing loop terminates
at the first unescaped ')', so a payload like "users)(hidden=*" collapses to
"(name=users)" with the trailing bytes silently discarded. An escaped "\)" makes
ldb_parse_tree() return NULL rather than admit injection. Combined with
LDB_SCOPE_ONELEVEL, the attribute list restricted to ["name"], and the fact that
a DRSUAPI-authenticated client can already issue arbitrary LDAP searches via
dcesrv_samdb_connect_as_user(), there is no escalation surface.

So this is only hardening helping AI to avoid reporting
false positives.

This was reported by Arjun Basnet with Securin Labs.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>

diff --git a/source4/dsdb/samdb/cracknames.c b/source4/dsdb/samdb/cracknames.c
index 65752ac45a73771491ed93a8e1b64266cffc6234..7fd69b6ac857e7a63cd56d2bc6be87d51b0f67a9 100644 (file)
--- a/source4/dsdb/samdb/cracknames.c
+++ b/source4/dsdb/samdb/cracknames.c
@@ -441,6 +441,9 @@ static WERROR get_format_functional_filtering_param(struct ldb_context *sam_ctx,
                        s[0] = '\0';
                        s++;
 
+                       account = ldb_binary_encode_string(mem_ctx, account);
+                       W_ERROR_HAVE_NO_MEMORY(account);
+
                        ldb_ret = ldb_search(sam_ctx, mem_ctx, &domain_res,
                                                tmp_dn,
                                                LDB_SCOPE_ONELEVEL,