From 2d9a2c3138907e789a1fa9b25c8636ad871314fd Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 9 May 2023 16:10:59 +1200 Subject: [PATCH] s4:dsdb: Check ldb_binary_encode_string() return value Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- source4/dsdb/common/util.c | 26 +++++-- source4/dsdb/common/util_samr.c | 27 ++++++- source4/dsdb/repl/drepl_partitions.c | 9 ++- source4/dsdb/samdb/cracknames.c | 86 +++++++++++++++++++---- source4/dsdb/samdb/ldb_modules/netlogon.c | 8 ++- 5 files changed, 132 insertions(+), 24 deletions(-) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 856e9401e66..38ad7eaf1fd 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -3062,6 +3062,12 @@ struct ldb_dn *samdb_dns_domain_to_dn(struct ldb_context *ldb, TALLOC_CTX *mem_c dn = ldb_dn_new(mem_ctx, ldb, NULL); for (i=0; split_realm[i]; i++) { binary_encoded = ldb_binary_encode_string(tmp_ctx, split_realm[i]); + if (binary_encoded == NULL) { + DEBUG(2, ("Failed to add dc= element to DN %s\n", + ldb_dn_get_linearized(dn))); + talloc_free(tmp_ctx); + return NULL; + } if (!ldb_dn_add_base_fmt(dn, "dc=%s", binary_encoded)) { DEBUG(2, ("Failed to add dc=%s element to DN %s\n", binary_encoded, ldb_dn_get_linearized(dn))); @@ -3164,14 +3170,20 @@ struct ldb_dn *samdb_domain_to_dn(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, }; struct ldb_result *res_domain_ref; char *escaped_domain = ldb_binary_encode_string(mem_ctx, domain_name); + int ret_domain; + + if (escaped_domain == NULL) { + return NULL; + } + /* find the domain's DN */ - int ret_domain = ldb_search(ldb, mem_ctx, - &res_domain_ref, - samdb_partitions_dn(ldb, mem_ctx), - LDB_SCOPE_ONELEVEL, - domain_ref_attrs, - "(&(nETBIOSName=%s)(objectclass=crossRef))", - escaped_domain); + ret_domain = ldb_search(ldb, mem_ctx, + &res_domain_ref, + samdb_partitions_dn(ldb, mem_ctx), + LDB_SCOPE_ONELEVEL, + domain_ref_attrs, + "(&(nETBIOSName=%s)(objectclass=crossRef))", + escaped_domain); if (ret_domain != LDB_SUCCESS) { return NULL; } diff --git a/source4/dsdb/common/util_samr.c b/source4/dsdb/common/util_samr.c index 89339c109d0..cf39b35846c 100644 --- a/source4/dsdb/common/util_samr.c +++ b/source4/dsdb/common/util_samr.c @@ -57,9 +57,17 @@ NTSTATUS dsdb_add_user(struct ldb_context *ldb, struct ldb_dn *account_dn; struct dom_sid *account_sid; + const char *account_name_encoded = NULL; + TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx); NT_STATUS_HAVE_NO_MEMORY(tmp_ctx); + account_name_encoded = ldb_binary_encode_string(tmp_ctx, account_name); + if (account_name_encoded == NULL) { + talloc_free(tmp_ctx); + return NT_STATUS_NO_MEMORY; + } + /* * Start a transaction, so we can query and do a subsequent atomic * modify @@ -77,7 +85,7 @@ NTSTATUS dsdb_add_user(struct ldb_context *ldb, name = samdb_search_string(ldb, tmp_ctx, NULL, "sAMAccountName", "(&(sAMAccountName=%s)(objectclass=user))", - ldb_binary_encode_string(tmp_ctx, account_name)); + account_name_encoded); if (name != NULL) { ldb_transaction_cancel(ldb); talloc_free(tmp_ctx); @@ -265,16 +273,23 @@ NTSTATUS dsdb_add_domain_group(struct ldb_context *ldb, const char *name; struct ldb_message *msg; struct dom_sid *group_sid; + const char *groupname_encoded = NULL; int ret; TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx); NT_STATUS_HAVE_NO_MEMORY(tmp_ctx); + groupname_encoded = ldb_binary_encode_string(tmp_ctx, groupname); + if (groupname_encoded == NULL) { + talloc_free(tmp_ctx); + return NT_STATUS_NO_MEMORY; + } + /* check if the group already exists */ name = samdb_search_string(ldb, tmp_ctx, NULL, "sAMAccountName", "(&(sAMAccountName=%s)(objectclass=group))", - ldb_binary_encode_string(tmp_ctx, groupname)); + groupname_encoded); if (name != NULL) { return NT_STATUS_GROUP_EXISTS; } @@ -341,11 +356,17 @@ NTSTATUS dsdb_add_domain_alias(struct ldb_context *ldb, const char *name; struct ldb_message *msg; struct dom_sid *alias_sid; + const char *alias_name_encoded = NULL; int ret; TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx); NT_STATUS_HAVE_NO_MEMORY(tmp_ctx); + alias_name_encoded = ldb_binary_encode_string(tmp_ctx, alias_name); + if (alias_name_encoded == NULL) { + return NT_STATUS_NO_MEMORY; + } + if (ldb_transaction_start(ldb) != LDB_SUCCESS) { DEBUG(0, ("Failed to start transaction in dsdb_add_domain_alias(): %s\n", ldb_errstring(ldb))); return NT_STATUS_INTERNAL_ERROR; @@ -355,7 +376,7 @@ NTSTATUS dsdb_add_domain_alias(struct ldb_context *ldb, name = samdb_search_string(ldb, tmp_ctx, NULL, "sAMAccountName", "(sAMAccountName=%s)(objectclass=group))", - ldb_binary_encode_string(mem_ctx, alias_name)); + alias_name_encoded); if (name != NULL) { talloc_free(tmp_ctx); diff --git a/source4/dsdb/repl/drepl_partitions.c b/source4/dsdb/repl/drepl_partitions.c index cc32d5904da..6d5f516a969 100644 --- a/source4/dsdb/repl/drepl_partitions.c +++ b/source4/dsdb/repl/drepl_partitions.c @@ -140,12 +140,19 @@ static bool dreplsrv_spn_exists(struct ldb_context *samdb, struct ldb_dn *accoun const char *attrs_empty[] = { NULL }; int ret; struct ldb_result *res; + const char *principal_name_encoded = NULL; tmp_ctx = talloc_new(samdb); + principal_name_encoded = ldb_binary_encode_string(tmp_ctx, principal_name); + if (principal_name_encoded == NULL) { + talloc_free(tmp_ctx); + return false; + } + ret = dsdb_search(samdb, tmp_ctx, &res, account_dn, LDB_SCOPE_BASE, attrs_empty, 0, "servicePrincipalName=%s", - ldb_binary_encode_string(tmp_ctx, principal_name)); + principal_name_encoded); if (ret != LDB_SUCCESS || res->count != 1) { talloc_free(tmp_ctx); return false; diff --git a/source4/dsdb/samdb/cracknames.c b/source4/dsdb/samdb/cracknames.c index 253d80cf59a..6ba232ca2b1 100644 --- a/source4/dsdb/samdb/cracknames.c +++ b/source4/dsdb/samdb/cracknames.c @@ -281,7 +281,9 @@ static WERROR DsCrackNameUPN(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, krb5_error_code ret; krb5_principal principal; char *realm; + char *realm_encoded = NULL; char *unparsed_name_short; + const char *unparsed_name_short_encoded = NULL; const char *domain_attrs[] = { NULL }; struct ldb_result *domain_res = NULL; @@ -301,15 +303,21 @@ static WERROR DsCrackNameUPN(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, realm = smb_krb5_principal_get_realm( mem_ctx, smb_krb5_context->krb5_context, principal); + realm_encoded = ldb_binary_encode_string(mem_ctx, realm); + if (realm_encoded == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } + ldb_ret = ldb_search(sam_ctx, mem_ctx, &domain_res, samdb_partitions_dn(sam_ctx, mem_ctx), LDB_SCOPE_ONELEVEL, domain_attrs, "(&(objectClass=crossRef)(|(dnsRoot=%s)(netbiosName=%s))" "(systemFlags:"LDB_OID_COMPARATOR_AND":=%u))", - ldb_binary_encode_string(mem_ctx, realm), - ldb_binary_encode_string(mem_ctx, realm), + realm_encoded, + realm_encoded, SYSTEM_FLAG_CR_NTDS_DOMAIN); + TALLOC_FREE(realm_encoded); TALLOC_FREE(realm); if (ldb_ret != LDB_SUCCESS) { @@ -349,9 +357,15 @@ static WERROR DsCrackNameUPN(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, return WERR_NOT_ENOUGH_MEMORY; } + unparsed_name_short_encoded = ldb_binary_encode_string(mem_ctx, unparsed_name_short); + if (unparsed_name_short_encoded == NULL) { + free(unparsed_name_short); + return WERR_NOT_ENOUGH_MEMORY; + } + /* This may need to be extended for more userPrincipalName variations */ result_filter = talloc_asprintf(mem_ctx, "(&(samAccountName=%s)(objectClass=user))", - ldb_binary_encode_string(mem_ctx, unparsed_name_short)); + unparsed_name_short_encoded); domain_filter = talloc_asprintf(mem_ctx, "(distinguishedName=%s)", ldb_dn_get_linearized(domain_res->msgs[0]->dn)); @@ -522,6 +536,7 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, case DRSUAPI_DS_NAME_FORMAT_CANONICAL_EX: { char *str, *s, *account; + const char *str_encoded = NULL; scope = LDB_SCOPE_ONELEVEL; if (strlen(name) == 0) { @@ -552,8 +567,13 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, s[0] = '\0'; s++; + str_encoded = ldb_binary_encode_string(mem_ctx, str); + if (str_encoded == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } + domain_filter = talloc_asprintf(mem_ctx, "(&(objectClass=crossRef)(dnsRoot=%s)(systemFlags:%s:=%u))", - ldb_binary_encode_string(mem_ctx, str), + str_encoded, LDB_OID_COMPARATOR_AND, SYSTEM_FLAG_CR_NTDS_DOMAIN); W_ERROR_HAVE_NO_MEMORY(domain_filter); @@ -579,6 +599,7 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, case DRSUAPI_DS_NAME_FORMAT_NT4_ACCOUNT: { char *p; char *domain; + char *domain_encoded = NULL; const char *account = NULL; domain = talloc_strdup(mem_ctx, name); @@ -596,15 +617,27 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, account = &p[1]; } + domain_encoded = ldb_binary_encode_string(mem_ctx, domain); + if (domain_encoded == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } + domain_filter = talloc_asprintf(mem_ctx, "(&(objectClass=crossRef)(netbiosName=%s)(systemFlags:%s:=%u))", - ldb_binary_encode_string(mem_ctx, domain), + domain_encoded, LDB_OID_COMPARATOR_AND, SYSTEM_FLAG_CR_NTDS_DOMAIN); W_ERROR_HAVE_NO_MEMORY(domain_filter); if (account) { + const char *account_encoded = NULL; + + account_encoded = ldb_binary_encode_string(mem_ctx, account); + if (account_encoded == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } + result_filter = talloc_asprintf(mem_ctx, "(sAMAccountName=%s)", - ldb_binary_encode_string(mem_ctx, account)); + account_encoded); W_ERROR_HAVE_NO_MEMORY(result_filter); } @@ -646,11 +679,18 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, break; } case DRSUAPI_DS_NAME_FORMAT_DISPLAY: { + const char *name_encoded = NULL; + domain_filter = NULL; + name_encoded = ldb_binary_encode_string(mem_ctx, name); + if (name_encoded == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } + result_filter = talloc_asprintf(mem_ctx, "(|(displayName=%s)(samAccountName=%s))", - ldb_binary_encode_string(mem_ctx, name), - ldb_binary_encode_string(mem_ctx, name)); + name_encoded, + name_encoded); W_ERROR_HAVE_NO_MEMORY(result_filter); break; } @@ -679,6 +719,7 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, case DRSUAPI_DS_NAME_FORMAT_USER_PRINCIPAL: { krb5_principal principal; char *unparsed_name; + const char *unparsed_name_encoded = NULL; ret = smb_krb5_init_context(mem_ctx, (struct loadparm_context *)ldb_get_opaque(sam_ctx, "loadparm"), @@ -716,9 +757,14 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, krb5_free_principal(smb_krb5_context->krb5_context, principal); + unparsed_name_encoded = ldb_binary_encode_string(mem_ctx, unparsed_name); + if (unparsed_name_encoded == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } + /* The ldb_binary_encode_string() here avoid LDAP filter injection attacks */ result_filter = talloc_asprintf(mem_ctx, "(&(userPrincipalName=%s)(objectClass=user))", - ldb_binary_encode_string(mem_ctx, unparsed_name)); + unparsed_name_encoded); free(unparsed_name); W_ERROR_HAVE_NO_MEMORY(result_filter); @@ -727,6 +773,7 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, case DRSUAPI_DS_NAME_FORMAT_SERVICE_PRINCIPAL: { krb5_principal principal; char *unparsed_name_short; + const char *unparsed_name_short_encoded = NULL; const krb5_data *component; char *service; @@ -764,6 +811,13 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, return WERR_NOT_ENOUGH_MEMORY; } + unparsed_name_short_encoded = ldb_binary_encode_string(mem_ctx, unparsed_name_short); + if (unparsed_name_short_encoded == NULL) { + krb5_free_principal(smb_krb5_context->krb5_context, principal); + free(unparsed_name_short); + return WERR_NOT_ENOUGH_MEMORY; + } + component = krb5_princ_component(smb_krb5_context->krb5_context, principal, 0); service = (char *)component->data; @@ -772,6 +826,7 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, (strcasecmp(service, "host") == 0)) { /* the 'cn' attribute is just the leading part of the name */ char *computer_name; + const char *computer_name_encoded = NULL; component = krb5_princ_component( smb_krb5_context->krb5_context, principal, 1); @@ -783,12 +838,19 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, return WERR_NOT_ENOUGH_MEMORY; } + computer_name_encoded = ldb_binary_encode_string(mem_ctx, computer_name); + if (computer_name_encoded == NULL) { + krb5_free_principal(smb_krb5_context->krb5_context, principal); + free(unparsed_name_short); + return WERR_NOT_ENOUGH_MEMORY; + } + result_filter = talloc_asprintf(mem_ctx, "(|(&(servicePrincipalName=%s)(objectClass=user))(&(cn=%s)(objectClass=computer)))", - ldb_binary_encode_string(mem_ctx, unparsed_name_short), - ldb_binary_encode_string(mem_ctx, computer_name)); + unparsed_name_short_encoded, + computer_name_encoded); } else { result_filter = talloc_asprintf(mem_ctx, "(&(servicePrincipalName=%s)(objectClass=user))", - ldb_binary_encode_string(mem_ctx, unparsed_name_short)); + unparsed_name_short_encoded); } krb5_free_principal(smb_krb5_context->krb5_context, principal); free(unparsed_name_short); diff --git a/source4/dsdb/samdb/ldb_modules/netlogon.c b/source4/dsdb/samdb/ldb_modules/netlogon.c index 4864ae4740e..479b3a6deec 100644 --- a/source4/dsdb/samdb/ldb_modules/netlogon.c +++ b/source4/dsdb/samdb/ldb_modules/netlogon.c @@ -237,6 +237,12 @@ NTSTATUS fill_netlogon_samlogon_response(struct ldb_context *sam_ctx, } } else if (user[0]) { struct ldb_result *user_res = NULL; + const char *user_encoded = NULL; + + user_encoded = ldb_binary_encode_string(mem_ctx, user); + if (user_encoded == NULL) { + return NT_STATUS_NO_MEMORY; + } /* We must exclude disabled accounts, but otherwise do the bitwise match the client asked for */ ret = ldb_search(sam_ctx, mem_ctx, &user_res, @@ -245,7 +251,7 @@ NTSTATUS fill_netlogon_samlogon_response(struct ldb_context *sam_ctx, "(&(objectClass=user)(samAccountName=%s)" "(!(userAccountControl:" LDB_OID_COMPARATOR_AND ":=%u))" "(userAccountControl:" LDB_OID_COMPARATOR_OR ":=%u))", - ldb_binary_encode_string(mem_ctx, user), + user_encoded, UF_ACCOUNTDISABLE, uac); if (ret != LDB_SUCCESS) { DEBUG(2,("Unable to find reference to user '%s' with ACB 0x%8x under %s: %s\n", -- 2.34.1