Re: [stunnel-users] fix resource leaks and potential null dereferences in 4.54

You usually have to declare the code in public domain. On Jan 29, 2013 7:36 PM, "Arthur Mesh" arthurmesh@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@stunnel.org https://www.stunnel.org/cgi-bin/mailman/listinfo/stunnel-users