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