[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