[stunnel-users] fix resource leaks and potential null
Brian Wilkins
bwilkins at gmail.com
Mon Feb 4 20:48:36 CET 2013
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 at stunnel.org>
>
> On Sun, Feb 03, 2013 at 12:00:02PM +0100,
> stunnel-users-request at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.stunnel.org/pipermail/stunnel-users/attachments/20130204/c50f16df/attachment.html>
More information about the stunnel-users
mailing list