#85 Potential failure during instance initialization (unchecked return value)
Closed: Fixed None Opened 11 years ago by pspacek.

Coverity found string operation with unchecked return value:

ldap_helper.c:
 324isc_result_t
 325new_ldap_instance(isc_mem_t *mctx, const char *db_name,
 326                  const char * const *argv, dns_dyndb_arguments_t *dyndb_args,
 327                  isc_task_t *task, ldap_instance_t **ldap_instp)
 328{

...

At conditional (1): "ldap_instp != NULL" taking the true branch.
At conditional (2): "*ldap_instp == NULL" taking the true branch.
 358        REQUIRE(ldap_instp != NULL && *ldap_instp == NULL);
 359
 360        ldap_inst = isc_mem_get(mctx, sizeof(ldap_instance_t));
At conditional (3): "ldap_inst == NULL" taking the false branch.
 361        if (ldap_inst == NULL)
 362                return ISC_R_NOMEMORY;
 363
 364        ZERO_PTR(ldap_inst);
 365
 366        isc_mem_attach(mctx, &ldap_inst->mctx);
 367        ldap_inst->db_name = db_name;
 368        ldap_inst->view = dns_dyndb_get_view(dyndb_args);
 369        ldap_inst->zmgr = dns_dyndb_get_zonemgr(dyndb_args);
 370        /* commented out for now, cause named to hang */
 371        //dns_view_attach(view, &ldap_inst->view);
 372
At conditional (4): "result != 0U" taking the false branch.
 373        CHECK(zr_create(mctx, &ldap_inst->zone_register));
 374
At conditional (5): "result != 0U" taking the false branch.
 375        CHECK(isc_mutex_init(&ldap_inst->kinit_lock));
 376
At conditional (6): "result != 0U" taking the false branch.
 377        CHECK(str_new(mctx, &auth_method_str));
At conditional (7): "result != 0U" taking the false branch.
 378        CHECK(str_new(mctx, &ldap_inst->uri));
At conditional (8): "result != 0U" taking the false branch.
 379        CHECK(str_new(mctx, &ldap_inst->base));
At conditional (9): "result != 0U" taking the false branch.
 380        CHECK(str_new(mctx, &ldap_inst->bind_dn));
At conditional (10): "result != 0U" taking the false branch.
 381        CHECK(str_new(mctx, &ldap_inst->password));
At conditional (11): "result != 0U" taking the false branch.
 382        CHECK(str_new(mctx, &ldap_inst->krb5_principal));
At conditional (12): "result != 0U" taking the false branch.
 383        CHECK(str_new(mctx, &ldap_inst->sasl_mech));
At conditional (13): "result != 0U" taking the false branch.
 384        CHECK(str_new(mctx, &ldap_inst->sasl_user));
At conditional (14): "result != 0U" taking the false branch.
 385        CHECK(str_new(mctx, &ldap_inst->sasl_auth_name));
At conditional (15): "result != 0U" taking the false branch.
 386        CHECK(str_new(mctx, &ldap_inst->sasl_realm));
At conditional (16): "result != 0U" taking the false branch.
 387        CHECK(str_new(mctx, &ldap_inst->sasl_password));
At conditional (17): "result != 0U" taking the false branch.
 388        CHECK(str_new(mctx, &ldap_inst->krb5_keytab));
At conditional (18): "result != 0U" taking the false branch.
 389        CHECK(str_new(mctx, &ldap_inst->fake_mname));
At conditional (19): "result != 0U" taking the false branch.
 390        CHECK(str_new(mctx, &ldap_inst->ldap_hostname));
 391
 392        i = 0;
 393        ldap_settings[i++].target = ldap_inst->uri;
 394        ldap_settings[i++].target = &ldap_inst->connections;
 395        ldap_settings[i++].target = &ldap_inst->reconnect_interval;
 396        ldap_settings[i++].target = &ldap_inst->timeout;
 397        ldap_settings[i++].target = ldap_inst->base;
 398        ldap_settings[i++].target = auth_method_str;
 399        ldap_settings[i++].target = ldap_inst->bind_dn;
 400        ldap_settings[i++].target = ldap_inst->password;
 401        ldap_settings[i++].target = ldap_inst->krb5_principal;
 402        ldap_settings[i++].target = ldap_inst->sasl_mech;
 403        ldap_settings[i++].target = ldap_inst->sasl_user;
 404        ldap_settings[i++].target = ldap_inst->sasl_auth_name;
 405        ldap_settings[i++].target = ldap_inst->sasl_realm;
 406        ldap_settings[i++].target = ldap_inst->sasl_password;
 407        ldap_settings[i++].target = ldap_inst->krb5_keytab;
 408        ldap_settings[i++].target = ldap_inst->fake_mname;
 409        ldap_settings[i++].target = &ldap_inst->psearch; 
 410        ldap_settings[i++].target = ldap_inst->ldap_hostname;
 411        ldap_settings[i++].target = &ldap_inst->sync_ptr;
 412        ldap_settings[i++].target = &ldap_inst->dyn_update;
 413        ldap_settings[i++].target = &ldap_inst->serial_autoincrement;
At conditional (20): "result != 0U" taking the false branch.
 414        CHECK(set_settings(ldap_settings, argv));
 415
 416        /* Set timer for deadlock detection inside semaphore_wait_timed . */
At conditional (21): "semaphore_wait_timeout.seconds < ldap_inst->timeout * 6U" taking the true branch.
 417        if (semaphore_wait_timeout.seconds < ldap_inst->timeout*SEM_WAIT_TIMEOUT_MUL)
 418                semaphore_wait_timeout.seconds = ldap_inst->timeout*SEM_WAIT_TIMEOUT_MUL;
 419
 420        /* Validate and check settings. */
 421        str_toupper(ldap_inst->sasl_mech);
At conditional (22): "ldap_inst->connections < 1U" taking the false branch.
 422        if (ldap_inst->connections < 1) {
 423                log_error("at least one connection is required");
 424                result = ISC_R_FAILURE;
 425                goto cleanup;
 426        }
 427        /* Select authentication method. */
 428        ldap_inst->auth_method = AUTH_INVALID;
At conditional (23): "supported_ldap_auth[i].name != NULL" taking the true branch.
 429        for (i = 0; supported_ldap_auth[i].name != NULL; i++) {
At conditional (24): "!str_casecmp_char(auth_method_str, supported_ldap_auth[i].name)" taking the true branch.
 430                if (!str_casecmp_char(auth_method_str,
 431                                      supported_ldap_auth[i].name)) {
 432                        ldap_inst->auth_method = supported_ldap_auth[i].value;
 433                        break;
 434                }
 435        }
At conditional (25): "ldap_inst->auth_method == 0U" taking the false branch.
 436        if (ldap_inst->auth_method == AUTH_INVALID) {
 437                log_error("unknown authentication method '%s'",
 438                          str_buf(auth_method_str));
 439                result = ISC_R_FAILURE;
 440                goto cleanup;
 441        }
 442
 443        /* check we have the right data when SASL/GSSAPI is selected */
At conditional (26): "ldap_inst->auth_method == 3U" taking the true branch.
At conditional (27): "str_casecmp_char(ldap_inst->sasl_mech, "GSSAPI") == 0" taking the true branch.
 444        if ((ldap_inst->auth_method == AUTH_SASL) &&
 445             (str_casecmp_char(ldap_inst->sasl_mech, "GSSAPI") == 0)) {
At conditional (28): "ldap_inst->krb5_principal == NULL" taking the false branch.
At conditional (29): "str_len(ldap_inst->krb5_principal) == 0UL" taking the true branch.
 446                if ((ldap_inst->krb5_principal == NULL) ||
 447                    (str_len(ldap_inst->krb5_principal) == 0)) {
At conditional (30): "ldap_inst->sasl_user == NULL" taking the true branch.
 448                        if ((ldap_inst->sasl_user == NULL) ||
 449                            (str_len(ldap_inst->sasl_user) == 0)) {
 450                                char hostname[255];
At conditional (31): "gethostname(hostname, 255UL) != 0" taking the false branch.
 451                                if (gethostname(hostname, 255) != 0) {
 452                                        log_error("SASL mech GSSAPI defined but krb5_principal"
 453                                                "and sasl_user are empty. Could not get hostname");
 454                                        result = ISC_R_FAILURE;
 455                                        goto cleanup;
 456                                } else {
CID 12566: Unchecked return value (CHECKED_RETURN)Calling function "str_sprintf" without checking return value (as is done elsewhere 4 out of 5 times).
No check of the return value of "str_sprintf(ldap_inst->krb5_principal, "DNS/%s", hostname)".
 457                                        str_sprintf(ldap_inst->krb5_principal, "DNS/%s", hostname);
 458                                        log_debug(2, "SASL mech GSSAPI defined but krb5_principal"
 459                                                "and sasl_user are empty, using default %s",
 460                                                str_buf(ldap_inst->krb5_principal));

This error is very improbable and it is not (simply) testable.

Metadata Update from @pspacek:
- Issue assigned to pspacek
- Issue set to the milestone: 3.0 IPA

7 years ago

Login to comment on this ticket.

Metadata