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>