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