Looks like a false positive..checking if a char array is null in that manner is a well known convention.<span></span><br><br>On Monday, February 4, 2013, Arthur Mesh  wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
dereferences in 4.54<br>
Reply-To:<br>
In-Reply-To: <<a>mailman.1.1359889202.25775.stunnel-users@stunnel.org</a>><br>
<br>
On Sun, Feb 03, 2013 at 12:00:02PM +0100, <a>stunnel-users-request@stunnel.org</a> wrote:<br>
> > -    char line_text[CONFLINELEN], *errstr;<br>
> > +    char line_text[CONFLINELEN], *errstr = NULL;<br>
> Were you able to identify a case where it's used without<br>
> initialization?  This is interesting.<br>
> It would be a good idea to fix it there instead of implementing a<br>
> workaround here.<br>
<br>
Hi,<br>
<br>
I will provide the output of coverity run for these defect:<br>
<br>
int parse_conf(char *name, CONF_TYPE type) {<br>
1997    DISK_FILE *df;<br>
At (1): Declaring variable "errstr" without initializer.<br>
1998    char line_text[CONFLINELEN], *errstr;<br>
1999    char config_line[CONFLINELEN], *config_opt, *config_arg;<br>
2000    int line_number, i;<br>
2001    SERVICE_OPTIONS *section, *new_section;<br>
2002    static char *filename=NULL; /* a copy of config file name for reloading */<br>
2003#ifndef USE_WIN32<br>
2004    int fd;<br>
2005    char *tmpstr;<br>
2006#endif<br>
2007<br>
At (2): Condition "name", taking true branch<br>
2008    if(name) /* not reload */<br>
2009        filename=str_dup(name);<br>
2010<br>
At (3): Condition "type == 2U", taking true branch<br>
2011    s_log(LOG_NOTICE, "Reading configuration from %s %s",<br>
2012        type==CONF_FD ? "descriptor" : "file", filename);<br>
2013#ifndef USE_WIN32<br>
At (4): Condition "type == 2U", taking true branch<br>
2014    if(type==CONF_FD) { /* file descriptor */<br>
2015        fd=strtol(filename, &tmpstr, 10);<br>
At (5): Condition "tmpstr == filename", taking false branch<br>
At (6): Condition "*tmpstr", taking false branch<br>
2016        if(tmpstr==filename || *tmpstr) { /* not a number */<br>
2017            s_log(LOG_ERR, "Invalid file descriptor number");<br>
2018            print_syntax();<br>
2019            return 1;<br>
2020        }<br>
2021        df=file_fdopen(fd);<br>
At (7): Falling through to end of if statement<br>
2022    } else<br>
2023#endif<br>
2024        df=file_open(filename, 0);<br>
At (8): Condition "!df", taking false branch<br>
2025    if(!df) {<br>
2026        s_log(LOG_ERR, "Cannot read configuration");<br>
2027        if(type!=CONF_RELOAD)<br>
2028            print_syntax();<br>
2029        return 1;<br>
2030    }<br>
2031<br>
2032    memset(&new_global_options, 0, sizeof(GLOBAL_OPTIONS)); /* reset global options */<br>
2033    memset(&new_service_options, 0, sizeof(SERVICE_OPTIONS)); /* reset local options */<br>
2034    new_service_options.next=NULL;<br>
2035    section=&new_service_options;<br>
2036    parse_global_option(CMD_BEGIN, NULL, NULL);<br>
2037    parse_service_option(CMD_BEGIN, section, NULL, NULL);<br>
At (9): Condition "type != 0U", taking true branch<br>
2038    if(type!=CONF_RELOAD) { /* provide defaults for gui.c */<br>
2039        memcpy(&global_options, &new_global_options, sizeof(GLOBAL_OPTIONS));<br>
2040        memcpy(&service_options, &new_service_options, sizeof(SERVICE_OPTIONS));<br>
2041    }<br>
2042<br>
2043    line_number=0;<br>
At (10): Condition "file_getline(df, line_text, 16384) >= 0", taking true branch<br>
At (19): Condition "file_getline(df, line_text, 16384) >= 0", taking false branch<br>
2044    while(file_getline(df, line_text, CONFLINELEN)>=0) {<br>
2045        memcpy(config_line, line_text, CONFLINELEN);<br>
2046        ++line_number;<br>
2047        config_opt=config_line;<br>
At (11): Condition "__istype((unsigned char)*config_opt, 16384UL)", taking true branch<br>
At (12): Condition "__istype((unsigned char)*config_opt, 16384UL)", taking false branch<br>
2048        while(isspace((unsigned char)*config_opt))<br>
2049            ++config_opt; /* remove initial whitespaces */<br>
At (13): Condition "i >= 0", taking true branch<br>
At (14): Condition "__istype((unsigned char)config_opt[i], 16384UL)", taking true branch<br>
At (15): Condition "i >= 0", taking true branch<br>
At (16): Condition "__istype((unsigned char)config_opt[i], 16384UL)", taking false branch<br>
2050        for(i=strlen(config_opt)-1; i>=0 && isspace((unsigned char)config_opt[i]); --i)<br>
2051            config_opt[i]='\0'; /* remove trailing whitespaces */<br>
At (17): Condition "config_opt[0] == 0", taking true branch<br>
2052        if(config_opt[0]=='\0' || config_opt[0]=='#' || config_opt[0]==';') /* empty or comment */<br>
At (18): Continuing loop<br>
2053            continue;<br>
2054        if(config_opt[0]=='[' && config_opt[strlen(config_opt)-1]==']') { /* new section */<br>
2055            if(!new_service_options.next) {<br>
2056                errstr=parse_global_option(CMD_END, NULL, NULL);<br>
2057                if(errstr) {<br>
2058                    s_log(LOG_ERR, "Line %d: \"%s\": %s", line_number, line_text, errstr);<br>
2059                    file_close(df);<br>
2060                    return 1;<br>
2061                }<br>
2062            }<br>
2063            ++config_opt;<br>
2064            config_opt[strlen(config_opt)-1]='\0';<br>
2065            new_section=str_alloc(sizeof(SERVICE_OPTIONS));<br>
2066            memcpy(new_section, &new_service_options, sizeof(SERVICE_OPTIONS));<br>
2067            new_section->servname=str_dup(config_opt);<br>
2068            new_section->session=NULL;<br>
2069            new_section->next=NULL;<br>
2070            section->next=new_section;<br>
2071            section=new_section;<br>
2072            continue;<br>
2073        }<br>
2074        config_arg=strchr(config_line, '=');<br>
2075        if(!config_arg) {<br>
2076            s_log(LOG_ERR, "Line %d: \"%s\": No '=' found", line_number, line_text);<br>
2077            file_close(df);<br>
2078            return 1;<br>
2079        }<br>
2080        *config_arg++='\0'; /* split into option name and argument value */<br>
2081        for(i=strlen(config_opt)-1; i>=0 && isspace((unsigned char)config_opt[i]); --i)<br>
2082            config_opt[i]='\0'; /* remove trailing whitespaces */<br>
2083        while(isspace((unsigned char)*config_arg))<br>
2084            ++config_arg; /* remove initial whitespaces */<br>
2085        errstr=parse_service_option(CMD_EXEC, section, config_opt, config_arg);<br>
2086        if(!new_service_options.next && errstr==option_not_found)<br>
2087            errstr=parse_global_option(CMD_EXEC, config_opt, config_arg);<br>
2088        if(errstr) {<br>
2089            s_log(LOG_ERR, "Line %d: \"%s\": %s", line_number, line_text, errstr);<br>
2090            file_close(df);<br>
2091            return 1;<br>
2092        }<br>
2093    }<br>
2094    file_close(df);<br>
2095<br>
At (20): Condition "new_service_options.next", taking true branch<br>
2096    if(new_service_options.next) { /* daemon mode: initialize sections */<br>
At (21): Condition "section", taking false branch<br>
2097        for(section=new_service_options.next; section; section=section->next) {<br>
2098            s_log(LOG_INFO, "Initializing service [%s]", section->servname);<br>
2099            errstr=parse_service_option(CMD_END, section, NULL, NULL);<br>
2100            if(errstr)<br>
2101                break;<br>
2102        }<br>
At (22): Falling through to end of if statement<br>
2103    } else { /* inetd mode: need to initialize global options */<br>
2104        errstr=parse_global_option(CMD_END, NULL, NULL);<br>
2105        if(errstr) {<br>
2106            s_log(LOG_ERR, "Global options: %s", errstr);<br>
2107            return 1;<br>
2108        }<br>
2109        s_log(LOG_INFO, "Initializing inetd mode configuration");<br>
2110        section=&new_service_options;<br>
2111        errstr=parse_service_option(CMD_END, section, NULL, NULL);<br>
2112    }<br>
CID 32706: Uninitialized pointer read (UNINIT)At (23): Using uninitialized value "errstr".<br>
2113    if(errstr) {<br>
CID 32693: Dereference after null check (FORWARD_NULL) [select defect]<br>
2114        s_log(LOG_ERR, "Service [%s]: %s", section->servname, errstr);<br>
2115        return 1;<br>
2116    }<br>
<br>
> > -        s_log(LOG_ERR, "Service [%s]: %s", section->servname, errstr);<br>
> > +        s_log(LOG_ERR, "Service [%s]: %s", section ? section->servname : "",<br>
> > +              errstr);<br>
> Again it would be useful to fix the root cause instead of implementing a<br>
> workaround.<br>
<br>
<br>
Coverity output:<br>
<br>
At (1): Condition "name", taking true branch<br>
2008    if(name) /* not reload */<br>
2009        filename=str_dup(name);<br>
2010<br>
At (2): Condition "type == 2U", taking true branch<br>
2011    s_log(LOG_NOTICE, "Reading configuration from %s %s",<br>
2012        type==CONF_FD ? "descriptor" : "file", filename);<br>
2013#ifndef USE_WIN32<br>
At (3): Condition "type == 2U", taking true branch<br>
2014    if(type==CONF_FD) { /* file descriptor */<br>
2015        fd=strtol(filename, &tmpstr, 10);<br>
At (4): Condition "tmpstr == filename", taking false branch<br>
At (5): Condition "*tmpstr", taking false branch<br>
2016        if(tmpstr==filename || *tmpstr) { /* not a number */<br>
2017            s_log(LOG_ERR, "Invalid file descriptor number");<br>
2018            print_syntax();<br>
2019            return 1;<br>
2020        }<br>
2021        df=file_fdopen(fd);<br>
At (6): Falling through to end of if statement<br>
2022    } else<br>
2023#endif<br>
2024        df=file_open(filename, 0);<br>
At (7): Condition "!df", taking false branch<br>
2025    if(!df) {<br>
2026        s_log(LOG_ERR, "Cannot read configuration");<br>
2027        if(type!=CONF_RELOAD)<br>
2028            print_syntax();<br>
2029        return 1;<br>
2030    }<br>
2031<br>
2032    memset(&new_global_options, 0, sizeof(GLOBAL_OPTIONS)); /* reset global options */<br>
2033    memset(&new_service_options, 0, sizeof(SERVICE_OPTIONS)); /* reset local options */<br>
2034    new_service_options.next=NULL;<br>
2035    section=&new_service_options;<br>
2036    parse_global_option(CMD_BEGIN, NULL, NULL);<br>
2037    parse_service_option(CMD_BEGIN, section, NULL, NULL);<br>
At (8): Condition "type != 0U", taking true branch<br>
2038    if(type!=CONF_RELOAD) { /* provide defaults for gui.c */<br>
2039        memcpy(&global_options, &new_global_options, sizeof(GLOBAL_OPTIONS));<br>
2040        memcpy(&service_options, &new_service_options, sizeof(SERVICE_OPTIONS));<br>
2041    }<br>
2042<br>
2043    line_number=0;<br>
At (9): Condition "file_getline(df, line_text, 16384) >= 0", taking true branch<br>
At (18): Condition "file_getline(df, line_text, 16384) >= 0", taking false branch<br>
2044    while(file_getline(df, line_text, CONFLINELEN)>=0) {<br>
2045        memcpy(config_line, line_text, CONFLINELEN);<br>
2046        ++line_number;<br>
2047        config_opt=config_line;<br>
At (10): Condition "__istype((unsigned char)*config_opt, 16384UL)", taking true branch<br>
At (11): Condition "__istype((unsigned char)*config_opt, 16384UL)", taking false branch<br>
2048        while(isspace((unsigned char)*config_opt))<br>
2049            ++config_opt; /* remove initial whitespaces */<br>
At (12): Condition "i >= 0", taking true branch<br>
At (13): Condition "__istype((unsigned char)config_opt[i], 16384UL)", taking true branch<br>
At (14): Condition "i >= 0", taking true branch<br>
At (15): Condition "__istype((unsigned char)config_opt[i], 16384UL)", taking false branch<br>
2050        for(i=strlen(config_opt)-1; i>=0 && isspace((unsigned char)config_opt[i]); --i)<br>
2051            config_opt[i]='\0'; /* remove trailing whitespaces */<br>
At (16): Condition "config_opt[0] == 0", taking true branch<br>
2052        if(config_opt[0]=='\0' || config_opt[0]=='#' || config_opt[0]==';') /* empty or comment */<br>
At (17): Continuing loop<br>
2053            continue;<br>
2054        if(config_opt[0]=='[' && config_opt[strlen(config_opt)-1]==']') { /* new section */<br>
2055            if(!new_service_options.next) {<br>
2056                errstr=parse_global_option(CMD_END, NULL, NULL);<br>
2057                if(errstr) {<br>
2058                    s_log(LOG_ERR, "Line %d: \"%s\": %s", line_number, line_text, errstr);<br>
2059                    file_close(df);<br>
2060                    return 1;<br>
2061                }<br>
2062            }<br>
2063            ++config_opt;<br>
2064            config_opt[strlen(config_opt)-1]='\0';<br>
2065            new_section=str_alloc(sizeof(SERVICE_OPTIONS));<br>
2066            memcpy(new_section, &new_service_options, sizeof(SERVICE_OPTIONS));<br>
2067            new_section->servname=str_dup(config_opt);<br>
2068            new_section->session=NULL;<br>
2069            new_section->next=NULL;<br>
2070            section->next=new_section;<br>
2071            section=new_section;<br>
2072            continue;<br>
2073        }<br>
2074        config_arg=strchr(config_line, '=');<br>
2075        if(!config_arg) {<br>
2076            s_log(LOG_ERR, "Line %d: \"%s\": No '=' found", line_number, line_text);<br>
2077            file_close(df);<br>
2078            return 1;<br>
2079        }<br>
2080        *config_arg++='\0'; /* split into option name and argument value */<br>
2081        for(i=strlen(config_opt)-1; i>=0 && isspace((unsigned char)config_opt[i]); --i)<br>
2082            config_opt[i]='\0'; /* remove trailing whitespaces */<br>
2083        while(isspace((unsigned char)*config_arg))<br>
2084            ++config_arg; /* remove initial whitespaces */<br>
2085        errstr=parse_service_option(CMD_EXEC, section, config_opt, config_arg);<br>
2086        if(!new_service_options.next && errstr==option_not_found)<br>
2087            errstr=parse_global_option(CMD_EXEC, config_opt, config_arg);<br>
2088        if(errstr) {<br>
2089            s_log(LOG_ERR, "Line %d: \"%s\": %s", line_number, line_text, errstr);<br>
2090            file_close(df);<br>
2091            return 1;<br>
2092        }<br>
2093    }<br>
2094    file_close(df);<br>
2095<br>
At (19): Condition "new_service_options.next", taking true branch<br>
2096    if(new_service_options.next) { /* daemon mode: initialize sections */<br>
At (20): Comparing "section" to null implies that "section" might be null.<br>
At (21): Condition</blockquote>