s4:dsdb: Check ldb_binary_encode_string() return value

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 856e9401e66fce27ea7dd3c2ca693ebe6d374270..38ad7eaf1fd861362fc96d632746d00848a2be6e 100644 (file)
--- 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 89339c109d0b55d790eb5f721402ae0502cd3e6c..cf39b35846c12aa915eb28322c73cb0411760d9b 100644 (file)
--- 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 cc32d5904da8e59decad3d51111c8da288d962f0..6d5f516a969d301b9cd82218bbf7dbd51b0b4a54 100644 (file)
--- 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 253d80cf59a1621e6e533c31eb0789dc1c811c0d..6ba232ca2b168906a89edd07af24261ca0272ece 100644 (file)
--- 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 4864ae4740e012a96598c7f77bb7d6e4879c3ac8..479b3a6deec7655001de1a611a809ce22ce64f66 100644 (file)
--- 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",