dereferences in 4.54
Reply-To:
In-Reply-To: <mailman.1.1359889202.25775.stunnel-users(a)stunnel.org>
On Sun, Feb 03, 2013 at 12:00:02PM +0100, stunnel-users-request(a)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 "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) [select defect]
At (23): Condition "errstr", taking true branch
2113 if(errstr) {
CID 32693: Dereference after null check (FORWARD_NULL)At (24): Dereferencing null pointer "section".
2114 s_log(LOG_ERR, "Service [%s]: %s", section->servname, errstr);
2115 return 1;
2116 }
> > - if(!strcasecmp(fl->name, string)) {
> > + if(string && !strcasecmp(fl->name, string)) {
> Could you give an example parameter where "string" may be NULL here?
2238/* facilities only make sense on Unix */
2239#if !defined (USE_WIN32) && !defined (__vms)
At (1): Condition "strchr(string, 46)", taking true branch
2240 if(strchr(string, '.')) { /* we have a facility specified */
2241 new_global_options.facility=-1;
2242 string=strtok(arg_copy, "."); /* break it up */
2243
At (2): Condition "fl->name", taking true branch
2244 for(fl=facilities; fl->name; ++fl) {
At (3): Condition "!strcasecmp(fl->name, string)", taking true branch
2245 if(!strcasecmp(fl->name, string)) {
2246 new_global_options.facility=fl->value;
At (4): Breaking from loop
2247 break;
2248 }
2249 }
At (5): Condition "new_global_options.facility == -1", taking false branch
2250 if(new_global_options.facility==-1)
2251 return 1; /* FAILED */
2252 string=strtok(NULL, "."); /* set to the remainder */
2253 }
2254#endif /* USE_WIN32, __vms */
2255
2256 /* time to check the syslog level */
At (6): Comparing "string" to null implies that "string" might be null.
At (7): Condition "string", taking false branch
2257 if(string && strlen(string)==1 && *string>='0' && *string<='7') {
2258 new_global_options.debug_level=*string-'0';
2259 return 0; /* OK */
2260 }
2261 new_global_options.debug_level=8; /* illegal level */
At (8): Condition "fl->name", taking true branch
2262 for(fl=levels; fl->name; ++fl) {
CID 11183: Dereference after null check (FORWARD_NULL)At (9): Passing null pointer "string" to function "strcasecmp(char const *, char const *)", which dereferences it.
2263 if(!strcasecmp(fl->name, string)) {
2264 new_global_options.debug_level=fl->value;
2265 break;
2266 }
2267 }
2268 if(new_global_options.debug_level==8)
2269 return 1; /* FAILED */
2270 return 0; /* OK */
2271}
Let me know if more info is required.
Thanks
