#45 configure_zone_forwarders returns a locally-scoped variable
Closed: Fixed None Opened 12 years ago by sgallagh.

This is a memory-corruption bug. You need to copy the value of address into addrs. It can't be referenced outside of the FOR loop.

Discovered by Coverity Integrity Manager scan.

 850static isc_result_t
 851configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst, 
 852                          dns_name_t *name, ldap_valuelist_t *values) {
 853                const char *dn = entry->dn;
 854                isc_result_t result;
 855                ldap_value_t *value;
 856                isc_sockaddrlist_t addrs;
 857
 858                REQUIRE(entry !=NULL && inst !=NULL && name != NULL && values != NULL);
 859
 860                /* Clean old fwdtable. */
 861                result = dns_fwdtable_delete(inst->view->fwdtable, name);
 862                if (result != ISC_R_SUCCESS) {
 863                        log_error("Failed to update forwarders");
 864                        return ISC_R_FAILURE;
 865                }
 866                
 867                ISC_LIST_INIT(addrs);
At conditional (1): "value != NULL" taking the true branch.
 868                for (value = HEAD(*values);
 869                     value != NULL;
 870                     value = NEXT(value, link)) {
 871                        isc_sockaddr_t address;
 872                        struct sockaddr sa;
 873
 874                        result = parse_nameserver(value->value, &sa);
At conditional (2): "result != 0U" taking the false branch.
 875                        if (result != ISC_R_SUCCESS) {
 876                                log_bug("Could not convert IP address from string '%s'.", value->value);
 877                        }
 878
 879                        /* Convert port from network byte order. */
At conditional (3): "0" taking the false branch.
 880                        in_port_t port = ntohs(get_in_port(&sa));
At conditional (4): "port != 0" taking the true branch.
 881                        port = (port != 0)?port:53; /* use well known port */                   
 882
 883                        isc_sockaddr_fromin(&address, get_in_addr(&sa), port);
 884                        ISC_LINK_INIT(&address, link);
CID 12384: Pointer to local outside scope (RETURN_LOCAL)Assigning: "addrs.tail" = "&address" (address of local variable "address").
Using "addrs.tail", which points to an out-of-scope variable "address".
 885                        ISC_LIST_APPEND(addrs, &address, link);
 886                        log_debug(5, "Adding forwarder %s (:%d) for %s", value->value, port, dn);
Variable "address" goes out of scope.
 887                }
 888
 889                /*
 890                 * Fetch forward policy.
 891                 */
 892                dns_fwdpolicy_t fwdpolicy = dns_fwdpolicy_first; /* "first" is default option. */
 893                result = ldap_entry_getvalues(entry, "idnsForwardPolicy", values);
 894                if (result == ISC_R_SUCCESS) {
 895                        value = HEAD(*values);
 896                        /*
 897                         * fwdpolicy: "only" or "first" (default)
 898                         */
 899                        if (value != NULL && value->value != NULL && strcmp(value->value, "only") == 0) {
 900                                fwdpolicy = dns_fwdpolicy_only;
 901                        }
 902                }
 903                log_debug(5, "Forward policy: %d", fwdpolicy);
 904                
 905                /* Set forward table up. */
 906                return dns_fwdtable_add(inst->view->fwdtable, name, &addrs, fwdpolicy);
 907        
 908}

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

7 years ago

Login to comment on this ticket.

Metadata