[stunnel-users] fix resource leaks and potential null dereferences in 4.54
Arthur Mesh
arthurmesh at gmail.com
Wed Jan 30 01:36:24 CET 2013
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
More information about the stunnel-users
mailing list