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