Hi,
I am trying to figure out where bug reports should be submitted. https://www.stunnel.org/lists.html references this mailing list, so here is my attempt at a bug report with a potential fix. If this is not an appropriate list, please redirect me.
A coverity run has uncovered quite a few memory and other resource leaks, plus some potential NULL dereferences. Inline is an attempt to fix them. Imho, these are mostly self-explanatory, but if someone needs more details, I would be happy to provide those as well.
Thanks
diff -ru stunnel-4.54/src/options.c stunnel-4.54.p/src/options.c --- stunnel-4.54/src/options.c 2012-10-09 01:50:54.000000000 -0700 +++ stunnel-4.54.p/src/options.c 2013-01-29 16:26:00.000000000 -0800 @@ -1988,7 +1988,7 @@
int parse_conf(char *name, CONF_TYPE type) { DISK_FILE *df; - char line_text[CONFLINELEN], *errstr; + char line_text[CONFLINELEN], *errstr = NULL; char config_line[CONFLINELEN], *config_opt, *config_arg; int line_number, i; SERVICE_OPTIONS *section, *new_section; @@ -2104,7 +2104,8 @@ errstr=parse_service_option(CMD_END, section, NULL, NULL); } if(errstr) { - s_log(LOG_ERR, "Service [%s]: %s", section->servname, errstr); + s_log(LOG_ERR, "Service [%s]: %s", section ? section->servname : "", + errstr); return 1; }
@@ -2253,7 +2254,7 @@ } new_global_options.debug_level=8; /* illegal level */ for(fl=levels; fl->name; ++fl) { - if(!strcasecmp(fl->name, string)) { + if(string && !strcasecmp(fl->name, string)) { new_global_options.debug_level=fl->value; break; } @@ -2428,6 +2429,7 @@ if(get_last_socket_error()!=S_ENOPROTOOPT) { s_log(LOG_ERR, "Failed to get %s OS default", ptr->opt_str); sockerror("getsockopt"); + close(fd); return 1; /* FAILED */ } td=str_dup("write-only"); @@ -2442,6 +2444,7 @@ ptr->opt_str, ta, tl, tr, td); str_free(ta); str_free(tl); str_free(tr); str_free(td); } + close(fd); return 0; /* OK */ }
Only in stunnel-4.54.p/src: options.c.orig diff -ru stunnel-4.54/src/protocol.c stunnel-4.54.p/src/protocol.c --- stunnel-4.54/src/protocol.c 2012-10-09 01:46:47.000000000 -0700 +++ stunnel-4.54.p/src/protocol.c 2013-01-29 16:26:00.000000000 -0800 @@ -244,30 +244,36 @@ /**************************************** smtp */
static void smtp_client(CLI *c) { - char *line; + char *line = NULL;
do { /* copy multiline greeting */ + str_free(line); /* okay to str_free(NULL) */ line=fd_getline(c, c->remote_fd.fd); fd_putline(c, c->local_wfd.fd, line); } while(isprefix(line, "220-"));
fd_putline(c, c->remote_fd.fd, "EHLO localhost"); do { /* skip multiline reply */ + str_free(line); line=fd_getline(c, c->remote_fd.fd); } while(isprefix(line, "250-")); if(!isprefix(line, "250 ")) { /* error */ + str_free(line); s_log(LOG_ERR, "Remote server is not RFC 1425 compliant"); longjmp(c->err, 1); }
fd_putline(c, c->remote_fd.fd, "STARTTLS"); do { /* skip multiline reply */ + str_free(line); line=fd_getline(c, c->remote_fd.fd); } while(isprefix(line, "220-")); if(!isprefix(line, "220 ")) { /* error */ + str_free(line); s_log(LOG_ERR, "Remote server is not RFC 2487 compliant"); longjmp(c->err, 1); } + str_free(line); }
static void smtp_server(CLI *c) { @@ -290,21 +296,27 @@ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "220")) { s_log(LOG_ERR, "Unknown server welcome"); + str_free(line); longjmp(c->err, 1); } fd_printf(c, c->local_wfd.fd, "%s + stunnel", line); + str_free(line); line=fd_getline(c, c->local_rfd.fd); if(!isprefix(line, "EHLO ")) { s_log(LOG_ERR, "Unknown client EHLO"); + str_free(line); longjmp(c->err, 1); } fd_printf(c, c->local_wfd.fd, "250-%s Welcome", line); + str_free(line); fd_putline(c, c->local_wfd.fd, "250 STARTTLS"); line=fd_getline(c, c->local_rfd.fd); if(!isprefix(line, "STARTTLS")) { s_log(LOG_ERR, "STARTTLS expected"); + str_free(line); longjmp(c->err, 1); } + str_free(line); fd_putline(c, c->local_wfd.fd, "220 Go ahead"); }
@@ -316,15 +328,19 @@ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "+OK ")) { s_log(LOG_ERR, "Unknown server welcome"); + str_free(line); longjmp(c->err, 1); } fd_putline(c, c->local_wfd.fd, line); + str_free(line); fd_putline(c, c->remote_fd.fd, "STLS"); line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "+OK ")) { s_log(LOG_ERR, "Server does not support TLS"); + str_free(line); longjmp(c->err, 1); } + str_free(line); }
static void pop3_server(CLI *c) { @@ -332,17 +348,21 @@
line=fd_getline(c, c->remote_fd.fd); fd_printf(c, c->local_wfd.fd, "%s + stunnel", line); + str_free(line); line=fd_getline(c, c->local_rfd.fd); if(isprefix(line, "CAPA")) { /* client wants RFC 2449 extensions */ fd_putline(c, c->local_wfd.fd, "+OK Stunnel capability list follows"); fd_putline(c, c->local_wfd.fd, "STLS"); fd_putline(c, c->local_wfd.fd, "."); + str_free(line); line=fd_getline(c, c->local_rfd.fd); } if(!isprefix(line, "STLS")) { + str_free(line); s_log(LOG_ERR, "Client does not want TLS"); longjmp(c->err, 1); } + str_free(line); fd_putline(c, c->local_wfd.fd, "+OK Stunnel starts TLS negotiation"); }
@@ -353,18 +373,22 @@
line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "* OK")) { + str_free(line); s_log(LOG_ERR, "Unknown server welcome"); longjmp(c->err, 1); } fd_putline(c, c->local_wfd.fd, line); + str_free(line); fd_putline(c, c->remote_fd.fd, "stunnel STARTTLS"); line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "stunnel OK")) { + str_free(line); fd_putline(c, c->local_wfd.fd, "* BYE stunnel: Server does not support TLS"); s_log(LOG_ERR, "Server does not support TLS"); longjmp(c->err, 2); /* don't reset */ } + str_free(line); }
static void imap_server(CLI *c) { @@ -387,6 +411,7 @@ /* process server welcome and send it to client */ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "* OK")) { + str_free(line); s_log(LOG_ERR, "Unknown server welcome"); longjmp(c->err, 1); } @@ -396,22 +421,29 @@ if(capa) *capa='K'; /* disable CAPABILITY within greeting */ fd_printf(c, c->local_wfd.fd, "%s (stunnel)", line); + str_free(line);
while(1) { /* process client commands */ line=fd_getline(c, c->local_rfd.fd); /* split line into id and tail */ id=str_dup(line); tail=strchr(id, ' '); - if(!tail) + if(!tail) { + str_free(line); + str_free(id); break; + } *tail++='\0';
if(isprefix(tail, "STARTTLS")) { fd_printf(c, c->local_wfd.fd, "%s OK Begin TLS negotiation now", id); + str_free(line); + str_free(id); return; /* success */ } else if(isprefix(tail, "CAPABILITY")) { fd_putline(c, c->remote_fd.fd, line); /* send it to server */ + str_free(line); line=fd_getline(c, c->remote_fd.fd); /* get the capabilites */ if(*line=='*') { /* @@ -421,6 +453,7 @@ * LOGIN would fail as "unexpected command", anyway */ fd_printf(c, c->local_wfd.fd, "%s STARTTLS", line); + str_free(line); line=fd_getline(c, c->remote_fd.fd); /* next line */ } fd_putline(c, c->local_wfd.fd, line); /* forward to the client */ @@ -429,24 +462,35 @@ fd_putline(c, c->local_wfd.fd, "* BYE unexpected server response"); s_log(LOG_ERR, "Unexpected server response: %s", line); + str_free(line); + str_free(id); break; } + str_free(line); + str_free(id); } else if(isprefix(tail, "LOGOUT")) { fd_putline(c, c->local_wfd.fd, "* BYE server terminating"); fd_printf(c, c->local_wfd.fd, "%s OK LOGOUT completed", id); + str_free(id); + str_free(line); break; } else { fd_putline(c, c->local_wfd.fd, "* BYE stunnel: unexpected command"); fd_printf(c, c->local_wfd.fd, "%s BAD %s unexpected", id, tail); s_log(LOG_ERR, "Unexpected client command %s", tail); + str_free(line); + str_free(id); break; } } /* clean server shutdown */ fd_putline(c, c->remote_fd.fd, "stunnel LOGOUT"); line=fd_getline(c, c->remote_fd.fd); - if(*line=='*') + if(*line=='*') { + str_free(line); line=fd_getline(c, c->remote_fd.fd); + } + str_free(line); longjmp(c->err, 2); /* don't reset */ }
@@ -458,15 +502,19 @@ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "200 ") && !isprefix(line, "201 ")) { s_log(LOG_ERR, "Unknown server welcome"); + str_free(line); longjmp(c->err, 1); } fd_putline(c, c->local_wfd.fd, line); fd_putline(c, c->remote_fd.fd, "STARTTLS"); + str_free(line); line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "382 ")) { s_log(LOG_ERR, "Server does not support TLS"); + str_free(line); longjmp(c->err, 1); } + str_free(line); }
/**************************************** connect */ @@ -548,13 +596,17 @@ /* not "HTTP/1.0 200 Connection established" */ s_log(LOG_ERR, "CONNECT request rejected"); do { /* read all headers */ + str_free(line); line=fd_getline(c, c->remote_fd.fd); } while(*line); + str_free(line); longjmp(c->err, 1); } + str_free(line); s_log(LOG_INFO, "CONNECT request accepted"); do { line=fd_getline(c, c->remote_fd.fd); /* read all headers */ + str_free(line); } while(*line); }
@@ -588,10 +640,13 @@ if(line[9]!='4' || line[10]!='0' || line[11]!='7') { /* code 407 */ s_log(LOG_ERR, "NTLM authorization request rejected"); do { /* read all headers */ + str_free(line); line=fd_getline(c, c->remote_fd.fd); } while(*line); + str_free(line); longjmp(c->err, 1); } + str_free(line); ntlm2_txt=NULL; do { /* read all headers */ line=fd_getline(c, c->remote_fd.fd); @@ -599,6 +654,7 @@ ntlm2_txt=str_dup(line+25); else if(isprefix(line, "Content-Length: ")) content_length=atol(line+16); + str_free(line); } while(*line); if(!ntlm2_txt) { /* no Proxy-Authenticate: NTLM header */ s_log(LOG_ERR, "Proxy-Authenticate: NTLM header not found"); Only in stunnel-4.54.p/src: protocol.c.orig diff -ru stunnel-4.54/src/stunnel.c stunnel-4.54.p/src/stunnel.c --- stunnel-4.54/src/stunnel.c 2012-08-18 14:11:53.000000000 -0700 +++ stunnel-4.54.p/src/stunnel.c 2013-01-29 16:26:00.000000000 -0800 @@ -119,12 +119,16 @@ fatal("Could not open /dev/null"); #endif /* standard Unix */ main_initialize(); - if(main_configure(argc>1 ? argv[1] : NULL, argc>2 ? argv[2] : NULL)) + if(main_configure(argc>1 ? argv[1] : NULL, argc>2 ? argv[2] : NULL)) { + close(fd); return 1; + } if(service_options.next) { /* there are service sections -> daemon mode */ #if !defined(__vms) && !defined(USE_OS2) - if(daemonize(fd)) + if(daemonize(fd)) { + close(fd); return 1; + } close(fd); /* create_pid() must be called after drop_privileges() * or it won't be possible to remove the file on exit */ Only in stunnel-4.54.p/src: stunnel.c.orig
You usually have to declare the code in public domain. On Jan 29, 2013 7:36 PM, "Arthur Mesh" [email protected] wrote:
Hi,
I am trying to figure out where bug reports should be submitted. https://www.stunnel.org/lists.html references this mailing list, so here is my attempt at a bug report with a potential fix. If this is not an appropriate list, please redirect me.
A coverity run has uncovered quite a few memory and other resource leaks, plus some potential NULL dereferences. Inline is an attempt to fix them. Imho, these are mostly self-explanatory, but if someone needs more details, I would be happy to provide those as well.
Thanks
diff -ru stunnel-4.54/src/options.c stunnel-4.54.p/src/options.c --- stunnel-4.54/src/options.c 2012-10-09 01:50:54.000000000 -0700 +++ stunnel-4.54.p/src/options.c 2013-01-29 16:26:00.000000000 -0800 @@ -1988,7 +1988,7 @@
int parse_conf(char *name, CONF_TYPE type) { DISK_FILE *df;
- char line_text[CONFLINELEN], *errstr;
- char line_text[CONFLINELEN], *errstr = NULL; char config_line[CONFLINELEN], *config_opt, *config_arg; int line_number, i; SERVICE_OPTIONS *section, *new_section;
@@ -2104,7 +2104,8 @@ errstr=parse_service_option(CMD_END, section, NULL, NULL); } if(errstr) {
s_log(LOG_ERR, "Service [%s]: %s", section->servname, errstr);
s_log(LOG_ERR, "Service [%s]: %s", section ? section->servname :"",
errstr); return 1;@@ -2253,7 +2254,7 @@ } new_global_options.debug_level=8; /* illegal level */ for(fl=levels; fl->name; ++fl) {
if(!strcasecmp(fl->name, string)) {
if(string && !strcasecmp(fl->name, string)) { new_global_options.debug_level=fl->value; break; }@@ -2428,6 +2429,7 @@ if(get_last_socket_error()!=S_ENOPROTOOPT) { s_log(LOG_ERR, "Failed to get %s OS default", ptr->opt_str); sockerror("getsockopt");
close(fd); return 1; /* FAILED */ } td=str_dup("write-only");@@ -2442,6 +2444,7 @@ ptr->opt_str, ta, tl, tr, td); str_free(ta); str_free(tl); str_free(tr); str_free(td); }
- close(fd); return 0; /* OK */
}
Only in stunnel-4.54.p/src: options.c.orig diff -ru stunnel-4.54/src/protocol.c stunnel-4.54.p/src/protocol.c --- stunnel-4.54/src/protocol.c 2012-10-09 01:46:47.000000000 -0700 +++ stunnel-4.54.p/src/protocol.c 2013-01-29 16:26:00.000000000 -0800 @@ -244,30 +244,36 @@ /**************************************** smtp */
static void smtp_client(CLI *c) {
- char *line;
char *line = NULL;
do { /* copy multiline greeting */
str_free(line); /* okay to str_free(NULL) */ line=fd_getline(c, c->remote_fd.fd); fd_putline(c, c->local_wfd.fd, line);} while(isprefix(line, "220-"));
fd_putline(c, c->remote_fd.fd, "EHLO localhost"); do { /* skip multiline reply */
str_free(line); line=fd_getline(c, c->remote_fd.fd);} while(isprefix(line, "250-")); if(!isprefix(line, "250 ")) { /* error */
str_free(line); s_log(LOG_ERR, "Remote server is not RFC 1425 compliant"); longjmp(c->err, 1);}
fd_putline(c, c->remote_fd.fd, "STARTTLS"); do { /* skip multiline reply */
str_free(line); line=fd_getline(c, c->remote_fd.fd);} while(isprefix(line, "220-")); if(!isprefix(line, "220 ")) { /* error */
str_free(line); s_log(LOG_ERR, "Remote server is not RFC 2487 compliant"); longjmp(c->err, 1);}
str_free(line);
}
static void smtp_server(CLI *c) { @@ -290,21 +296,27 @@ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "220")) { s_log(LOG_ERR, "Unknown server welcome");
str_free(line); longjmp(c->err, 1);- str_free(line); line=fd_getline(c, c->local_rfd.fd); if(!isprefix(line, "EHLO ")) { s_log(LOG_ERR, "Unknown client EHLO");
str_free(line); longjmp(c->err, 1);- str_free(line); fd_putline(c, c->local_wfd.fd, "250 STARTTLS"); line=fd_getline(c, c->local_rfd.fd); if(!isprefix(line, "STARTTLS")) { s_log(LOG_ERR, "STARTTLS expected");
str_free(line); longjmp(c->err, 1);- str_free(line); fd_putline(c, c->local_wfd.fd, "220 Go ahead");
}
@@ -316,15 +328,19 @@ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "+OK ")) { s_log(LOG_ERR, "Unknown server welcome");
str_free(line); longjmp(c->err, 1);- str_free(line); fd_putline(c, c->remote_fd.fd, "STLS"); line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "+OK ")) { s_log(LOG_ERR, "Server does not support TLS");
str_free(line); longjmp(c->err, 1);- str_free(line);
}
static void pop3_server(CLI *c) { @@ -332,17 +348,21 @@
line=fd_getline(c, c->remote_fd.fd); fd_printf(c, c->local_wfd.fd, "%s + stunnel", line);
- str_free(line); line=fd_getline(c, c->local_rfd.fd); if(isprefix(line, "CAPA")) { /* client wants RFC 2449 extensions */ fd_putline(c, c->local_wfd.fd, "+OK Stunnel capability list
follows"); fd_putline(c, c->local_wfd.fd, "STLS"); fd_putline(c, c->local_wfd.fd, ".");
str_free(line); line=fd_getline(c, c->local_rfd.fd);str_free(line); s_log(LOG_ERR, "Client does not want TLS"); longjmp(c->err, 1);- str_free(line); fd_putline(c, c->local_wfd.fd, "+OK Stunnel starts TLS negotiation");
}
@@ -353,18 +373,22 @@
line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "* OK")) {
str_free(line); s_log(LOG_ERR, "Unknown server welcome"); longjmp(c->err, 1);- str_free(line); fd_putline(c, c->remote_fd.fd, "stunnel STARTTLS"); line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "stunnel OK")) {
str_free(line); fd_putline(c, c->local_wfd.fd, "* BYE stunnel: Server does not support TLS"); s_log(LOG_ERR, "Server does not support TLS"); longjmp(c->err, 2); /* don't reset */- str_free(line);
}
static void imap_server(CLI *c) { @@ -387,6 +411,7 @@ /* process server welcome and send it to client */ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "* OK")) {
str_free(line); s_log(LOG_ERR, "Unknown server welcome"); longjmp(c->err, 1);@@ -396,22 +421,29 @@ if(capa) *capa='K'; /* disable CAPABILITY within greeting */ fd_printf(c, c->local_wfd.fd, "%s (stunnel)", line);
str_free(line);
while(1) { /* process client commands */ line=fd_getline(c, c->local_rfd.fd); /* split line into id and tail */ id=str_dup(line); tail=strchr(id, ' ');
if(!tail)
if(!tail) {str_free(line);str_free(id); break;} *tail++='\0'; if(isprefix(tail, "STARTTLS")) { fd_printf(c, c->local_wfd.fd, "%s OK Begin TLS negotiation now", id);str_free(line);str_free(id); return; /* success */ } else if(isprefix(tail, "CAPABILITY")) { fd_putline(c, c->remote_fd.fd, line); /* send it to server */str_free(line); line=fd_getline(c, c->remote_fd.fd); /* get the capabilites */ if(*line=='*') { /*@@ -421,6 +453,7 @@ * LOGIN would fail as "unexpected command", anyway */ fd_printf(c, c->local_wfd.fd, "%s STARTTLS", line);
str_free(line); line=fd_getline(c, c->remote_fd.fd); /* next line */ } fd_putline(c, c->local_wfd.fd, line); /* forward to theclient */ @@ -429,24 +462,35 @@ fd_putline(c, c->local_wfd.fd, "* BYE unexpected server response"); s_log(LOG_ERR, "Unexpected server response: %s", line);
str_free(line);str_free(id); break; }str_free(line);str_free(id); } else if(isprefix(tail, "LOGOUT")) { fd_putline(c, c->local_wfd.fd, "* BYE server terminating"); fd_printf(c, c->local_wfd.fd, "%s OK LOGOUT completed", id);str_free(id);str_free(line); break; } else { fd_putline(c, c->local_wfd.fd, "* BYE stunnel: unexpectedcommand"); fd_printf(c, c->local_wfd.fd, "%s BAD %s unexpected", id, tail); s_log(LOG_ERR, "Unexpected client command %s", tail);
str_free(line);str_free(id); break; }
- if(*line=='*')
- if(*line=='*') {
str_free(line); line=fd_getline(c, c->remote_fd.fd);- }
- str_free(line); longjmp(c->err, 2); /* don't reset */
}
@@ -458,15 +502,19 @@ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "200 ") && !isprefix(line, "201 ")) { s_log(LOG_ERR, "Unknown server welcome");
str_free(line); longjmp(c->err, 1);- str_free(line); line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "382 ")) { s_log(LOG_ERR, "Server does not support TLS");
str_free(line); longjmp(c->err, 1);- str_free(line);
}
/**************************************** connect */ @@ -548,13 +596,17 @@ /* not "HTTP/1.0 200 Connection established" */ s_log(LOG_ERR, "CONNECT request rejected"); do { /* read all headers */
str_free(line); line=fd_getline(c, c->remote_fd.fd); } while(*line);str_free(line); longjmp(c->err, 1);- str_free(line); s_log(LOG_INFO, "CONNECT request accepted"); do { line=fd_getline(c, c->remote_fd.fd); /* read all headers */
str_free(line);}
@@ -588,10 +640,13 @@ if(line[9]!='4' || line[10]!='0' || line[11]!='7') { /* code 407 */ s_log(LOG_ERR, "NTLM authorization request rejected"); do { /* read all headers */
str_free(line); line=fd_getline(c, c->remote_fd.fd); } while(*line);str_free(line); longjmp(c->err, 1);- str_free(line); ntlm2_txt=NULL; do { /* read all headers */ line=fd_getline(c, c->remote_fd.fd);
@@ -599,6 +654,7 @@ ntlm2_txt=str_dup(line+25); else if(isprefix(line, "Content-Length: ")) content_length=atol(line+16);
str_free(line);Only in stunnel-4.54.p/src: protocol.c.orig diff -ru stunnel-4.54/src/stunnel.c stunnel-4.54.p/src/stunnel.c --- stunnel-4.54/src/stunnel.c 2012-08-18 14:11:53.000000000 -0700 +++ stunnel-4.54.p/src/stunnel.c 2013-01-29 16:26:00.000000000 -0800 @@ -119,12 +119,16 @@ fatal("Could not open /dev/null"); #endif /* standard Unix */ main_initialize();
- if(main_configure(argc>1 ? argv[1] : NULL, argc>2 ? argv[2] : NULL))
- if(main_configure(argc>1 ? argv[1] : NULL, argc>2 ? argv[2] : NULL)) {
close(fd); return 1;- } if(service_options.next) { /* there are service sections -> daemon
mode */ #if !defined(__vms) && !defined(USE_OS2)
if(daemonize(fd))
if(daemonize(fd)) {close(fd); return 1;} close(fd); /* create_pid() must be called after drop_privileges() * or it won't be possible to remove the file on exit */Only in stunnel-4.54.p/src: stunnel.c.orig _______________________________________________ stunnel-users mailing list [email protected] https://www.stunnel.org/cgi-bin/mailman/listinfo/stunnel-users
On Tue, Jan 29, 2013 at 08:08:25PM -0500, Brian Wilkins wrote:
You usually have to declare the code in public domain.
Okay, here we go.
I, the copyright holder of this work, hereby release it into the public domain. This applies worldwide.
In case this is not legally possible, I grant any entity the right to use this work for any purpose, without any conditions, unless such conditions are required by law.
diff -ru stunnel-4.54/src/options.c stunnel-4.54.p/src/options.c --- stunnel-4.54/src/options.c 2012-10-09 01:50:54.000000000 -0700 +++ stunnel-4.54.p/src/options.c 2013-01-29 16:26:00.000000000 -0800 @@ -1988,7 +1988,7 @@
int parse_conf(char *name, CONF_TYPE type) { DISK_FILE *df; - char line_text[CONFLINELEN], *errstr; + char line_text[CONFLINELEN], *errstr = NULL; char config_line[CONFLINELEN], *config_opt, *config_arg; int line_number, i; SERVICE_OPTIONS *section, *new_section; @@ -2104,7 +2104,8 @@ errstr=parse_service_option(CMD_END, section, NULL, NULL); } if(errstr) { - s_log(LOG_ERR, "Service [%s]: %s", section->servname, errstr); + s_log(LOG_ERR, "Service [%s]: %s", section ? section->servname : "", + errstr); return 1; }
@@ -2253,7 +2254,7 @@ } new_global_options.debug_level=8; /* illegal level */ for(fl=levels; fl->name; ++fl) { - if(!strcasecmp(fl->name, string)) { + if(string && !strcasecmp(fl->name, string)) { new_global_options.debug_level=fl->value; break; } @@ -2428,6 +2429,7 @@ if(get_last_socket_error()!=S_ENOPROTOOPT) { s_log(LOG_ERR, "Failed to get %s OS default", ptr->opt_str); sockerror("getsockopt"); + close(fd); return 1; /* FAILED */ } td=str_dup("write-only"); @@ -2442,6 +2444,7 @@ ptr->opt_str, ta, tl, tr, td); str_free(ta); str_free(tl); str_free(tr); str_free(td); } + close(fd); return 0; /* OK */ }
Only in stunnel-4.54.p/src: options.c.orig diff -ru stunnel-4.54/src/protocol.c stunnel-4.54.p/src/protocol.c --- stunnel-4.54/src/protocol.c 2012-10-09 01:46:47.000000000 -0700 +++ stunnel-4.54.p/src/protocol.c 2013-01-29 16:26:00.000000000 -0800 @@ -244,30 +244,36 @@ /**************************************** smtp */
static void smtp_client(CLI *c) { - char *line; + char *line = NULL;
do { /* copy multiline greeting */ + str_free(line); /* okay to str_free(NULL) */ line=fd_getline(c, c->remote_fd.fd); fd_putline(c, c->local_wfd.fd, line); } while(isprefix(line, "220-"));
fd_putline(c, c->remote_fd.fd, "EHLO localhost"); do { /* skip multiline reply */ + str_free(line); line=fd_getline(c, c->remote_fd.fd); } while(isprefix(line, "250-")); if(!isprefix(line, "250 ")) { /* error */ + str_free(line); s_log(LOG_ERR, "Remote server is not RFC 1425 compliant"); longjmp(c->err, 1); }
fd_putline(c, c->remote_fd.fd, "STARTTLS"); do { /* skip multiline reply */ + str_free(line); line=fd_getline(c, c->remote_fd.fd); } while(isprefix(line, "220-")); if(!isprefix(line, "220 ")) { /* error */ + str_free(line); s_log(LOG_ERR, "Remote server is not RFC 2487 compliant"); longjmp(c->err, 1); } + str_free(line); }
static void smtp_server(CLI *c) { @@ -290,21 +296,27 @@ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "220")) { s_log(LOG_ERR, "Unknown server welcome"); + str_free(line); longjmp(c->err, 1); } fd_printf(c, c->local_wfd.fd, "%s + stunnel", line); + str_free(line); line=fd_getline(c, c->local_rfd.fd); if(!isprefix(line, "EHLO ")) { s_log(LOG_ERR, "Unknown client EHLO"); + str_free(line); longjmp(c->err, 1); } fd_printf(c, c->local_wfd.fd, "250-%s Welcome", line); + str_free(line); fd_putline(c, c->local_wfd.fd, "250 STARTTLS"); line=fd_getline(c, c->local_rfd.fd); if(!isprefix(line, "STARTTLS")) { s_log(LOG_ERR, "STARTTLS expected"); + str_free(line); longjmp(c->err, 1); } + str_free(line); fd_putline(c, c->local_wfd.fd, "220 Go ahead"); }
@@ -316,15 +328,19 @@ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "+OK ")) { s_log(LOG_ERR, "Unknown server welcome"); + str_free(line); longjmp(c->err, 1); } fd_putline(c, c->local_wfd.fd, line); + str_free(line); fd_putline(c, c->remote_fd.fd, "STLS"); line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "+OK ")) { s_log(LOG_ERR, "Server does not support TLS"); + str_free(line); longjmp(c->err, 1); } + str_free(line); }
static void pop3_server(CLI *c) { @@ -332,17 +348,21 @@
line=fd_getline(c, c->remote_fd.fd); fd_printf(c, c->local_wfd.fd, "%s + stunnel", line); + str_free(line); line=fd_getline(c, c->local_rfd.fd); if(isprefix(line, "CAPA")) { /* client wants RFC 2449 extensions */ fd_putline(c, c->local_wfd.fd, "+OK Stunnel capability list follows"); fd_putline(c, c->local_wfd.fd, "STLS"); fd_putline(c, c->local_wfd.fd, "."); + str_free(line); line=fd_getline(c, c->local_rfd.fd); } if(!isprefix(line, "STLS")) { + str_free(line); s_log(LOG_ERR, "Client does not want TLS"); longjmp(c->err, 1); } + str_free(line); fd_putline(c, c->local_wfd.fd, "+OK Stunnel starts TLS negotiation"); }
@@ -353,18 +373,22 @@
line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "* OK")) { + str_free(line); s_log(LOG_ERR, "Unknown server welcome"); longjmp(c->err, 1); } fd_putline(c, c->local_wfd.fd, line); + str_free(line); fd_putline(c, c->remote_fd.fd, "stunnel STARTTLS"); line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "stunnel OK")) { + str_free(line); fd_putline(c, c->local_wfd.fd, "* BYE stunnel: Server does not support TLS"); s_log(LOG_ERR, "Server does not support TLS"); longjmp(c->err, 2); /* don't reset */ } + str_free(line); }
static void imap_server(CLI *c) { @@ -387,6 +411,7 @@ /* process server welcome and send it to client */ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "* OK")) { + str_free(line); s_log(LOG_ERR, "Unknown server welcome"); longjmp(c->err, 1); } @@ -396,22 +421,29 @@ if(capa) *capa='K'; /* disable CAPABILITY within greeting */ fd_printf(c, c->local_wfd.fd, "%s (stunnel)", line); + str_free(line);
while(1) { /* process client commands */ line=fd_getline(c, c->local_rfd.fd); /* split line into id and tail */ id=str_dup(line); tail=strchr(id, ' '); - if(!tail) + if(!tail) { + str_free(line); + str_free(id); break; + } *tail++='\0';
if(isprefix(tail, "STARTTLS")) { fd_printf(c, c->local_wfd.fd, "%s OK Begin TLS negotiation now", id); + str_free(line); + str_free(id); return; /* success */ } else if(isprefix(tail, "CAPABILITY")) { fd_putline(c, c->remote_fd.fd, line); /* send it to server */ + str_free(line); line=fd_getline(c, c->remote_fd.fd); /* get the capabilites */ if(*line=='*') { /* @@ -421,6 +453,7 @@ * LOGIN would fail as "unexpected command", anyway */ fd_printf(c, c->local_wfd.fd, "%s STARTTLS", line); + str_free(line); line=fd_getline(c, c->remote_fd.fd); /* next line */ } fd_putline(c, c->local_wfd.fd, line); /* forward to the client */ @@ -429,24 +462,35 @@ fd_putline(c, c->local_wfd.fd, "* BYE unexpected server response"); s_log(LOG_ERR, "Unexpected server response: %s", line); + str_free(line); + str_free(id); break; } + str_free(line); + str_free(id); } else if(isprefix(tail, "LOGOUT")) { fd_putline(c, c->local_wfd.fd, "* BYE server terminating"); fd_printf(c, c->local_wfd.fd, "%s OK LOGOUT completed", id); + str_free(id); + str_free(line); break; } else { fd_putline(c, c->local_wfd.fd, "* BYE stunnel: unexpected command"); fd_printf(c, c->local_wfd.fd, "%s BAD %s unexpected", id, tail); s_log(LOG_ERR, "Unexpected client command %s", tail); + str_free(line); + str_free(id); break; } } /* clean server shutdown */ fd_putline(c, c->remote_fd.fd, "stunnel LOGOUT"); line=fd_getline(c, c->remote_fd.fd); - if(*line=='*') + if(*line=='*') { + str_free(line); line=fd_getline(c, c->remote_fd.fd); + } + str_free(line); longjmp(c->err, 2); /* don't reset */ }
@@ -458,15 +502,19 @@ line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "200 ") && !isprefix(line, "201 ")) { s_log(LOG_ERR, "Unknown server welcome"); + str_free(line); longjmp(c->err, 1); } fd_putline(c, c->local_wfd.fd, line); fd_putline(c, c->remote_fd.fd, "STARTTLS"); + str_free(line); line=fd_getline(c, c->remote_fd.fd); if(!isprefix(line, "382 ")) { s_log(LOG_ERR, "Server does not support TLS"); + str_free(line); longjmp(c->err, 1); } + str_free(line); }
/**************************************** connect */ @@ -548,13 +596,17 @@ /* not "HTTP/1.0 200 Connection established" */ s_log(LOG_ERR, "CONNECT request rejected"); do { /* read all headers */ + str_free(line); line=fd_getline(c, c->remote_fd.fd); } while(*line); + str_free(line); longjmp(c->err, 1); } + str_free(line); s_log(LOG_INFO, "CONNECT request accepted"); do { line=fd_getline(c, c->remote_fd.fd); /* read all headers */ + str_free(line); } while(*line); }
@@ -588,10 +640,13 @@ if(line[9]!='4' || line[10]!='0' || line[11]!='7') { /* code 407 */ s_log(LOG_ERR, "NTLM authorization request rejected"); do { /* read all headers */ + str_free(line); line=fd_getline(c, c->remote_fd.fd); } while(*line); + str_free(line); longjmp(c->err, 1); } + str_free(line); ntlm2_txt=NULL; do { /* read all headers */ line=fd_getline(c, c->remote_fd.fd); @@ -599,6 +654,7 @@ ntlm2_txt=str_dup(line+25); else if(isprefix(line, "Content-Length: ")) content_length=atol(line+16); + str_free(line); } while(*line); if(!ntlm2_txt) { /* no Proxy-Authenticate: NTLM header */ s_log(LOG_ERR, "Proxy-Authenticate: NTLM header not found"); Only in stunnel-4.54.p/src: protocol.c.orig diff -ru stunnel-4.54/src/stunnel.c stunnel-4.54.p/src/stunnel.c --- stunnel-4.54/src/stunnel.c 2012-08-18 14:11:53.000000000 -0700 +++ stunnel-4.54.p/src/stunnel.c 2013-01-29 16:26:00.000000000 -0800 @@ -119,12 +119,16 @@ fatal("Could not open /dev/null"); #endif /* standard Unix */ main_initialize(); - if(main_configure(argc>1 ? argv[1] : NULL, argc>2 ? argv[2] : NULL)) + if(main_configure(argc>1 ? argv[1] : NULL, argc>2 ? argv[2] : NULL)) { + close(fd); return 1; + } if(service_options.next) { /* there are service sections -> daemon mode */ #if !defined(__vms) && !defined(USE_OS2) - if(daemonize(fd)) + if(daemonize(fd)) { + close(fd); return 1; + } close(fd); /* create_pid() must be called after drop_privileges() * or it won't be possible to remove the file on exit */ Only in stunnel-4.54.p/src: stunnel.c.orig
On 2013-01-30 03:00, Arthur Mesh wrote:
I, the copyright holder of this work, hereby release it into the public domain. This applies worldwide.
In case this is not legally possible, I grant any entity the right to use this work for any purpose, without any conditions, unless such conditions are required by law.
Thank you.
- 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.
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.
if(!strcasecmp(fl->name, string)) {
if(string && !strcasecmp(fl->name, string)) {
Could you give an example parameter where "string" may be NULL here?
Mike
Many times these tools produce false positives. It takes a trained developerto spot them.
On Saturday, February 2, 2013, Michal Trojnara wrote:
On 2013-01-30 03:00, Arthur Mesh wrote:
I, the copyright holder of this work, hereby release it into the public domain. This applies worldwide.
In case this is not legally possible, I grant any entity the right to use this work for any purpose, without any conditions, unless such conditions are required by law.
Thank you.
- 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.
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.
if(!strcasecmp(fl->name, string)) {
if(string && !strcasecmp(fl->name, string)) {Could you give an example parameter where "string" may be NULL here?
Mike
-
Arthur Mesh -
Brian Wilkins -
Michal Trojnara