[stunnel-users] fix resource leaks and potential null dereferences in 4.54
Brian Wilkins
bwilkins at gmail.com
Wed Jan 30 02:08:25 CET 2013
You usually have to declare the code in public domain.
On Jan 29, 2013 7:36 PM, "Arthur Mesh" <arthurmesh at gmail.com> 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);
> }
> 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
> _______________________________________________
> stunnel-users mailing list
> stunnel-users at stunnel.org
> https://www.stunnel.org/cgi-bin/mailman/listinfo/stunnel-users
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.stunnel.org/pipermail/stunnel-users/attachments/20130129/1be97620/attachment.html>
More information about the stunnel-users
mailing list