be_openssl: avoid leaking of SSL structure

From nmathewson/Libevent#83 by @fancycode:
  There are a few code paths where the passed SSL object is not released in error cases, even if BEV_OPT_CLOSE_ON_FREE is passed as option while for others it is released. That way it's impossible for the caller to know it he has to free it on errors himself or not.

  Line numbers are from "bufferevent_openssl.c" in 911abf3:

  L1414 ("underlying == NULL" passed)
  L1416 (bio could not be created)
  L1446 (different fd passed)
  L1325 (both underlying and fd passed)
  L1328 (out-of-memory)
  L1333 ("bufferevent_init_common_" failed)
  In all error cases after the "bufferevent_ops_openssl" has been assigned, the option is evaluated on "bufferevent_free" (L1399) and the SSL object released (L1226).

Fixes: nmathewson/Libevent#83

diff --git a/bufferevent_openssl.c b/bufferevent_openssl.c
index c7f736d6014c45098e13b8487b5abe28e80054de..8e2b361c049deca3ec684b6ec85498499f606819 100644 (file)
--- a/bufferevent_openssl.c
+++ b/bufferevent_openssl.c
@@ -1348,8 +1348,9 @@ bufferevent_openssl_new_impl(struct event_base *base,
        struct bufferevent_private *bev_p = NULL;
        int tmp_options = options & ~BEV_OPT_THREADSAFE;
 
+       /* Only one can be set. */
        if (underlying != NULL && fd >= 0)
-               return NULL; /* Only one can be set. */
+               goto err;
 
        if (!(bev_ssl = mm_calloc(1, sizeof(struct bufferevent_openssl))))
                goto err;
@@ -1397,8 +1398,12 @@ bufferevent_openssl_new_impl(struct event_base *base,
 
        return &bev_ssl->bev.bev;
 err:
-       if (bev_ssl)
+       if (options & BEV_OPT_CLOSE_ON_FREE)
+               SSL_free(ssl);
+       if (bev_ssl) {
+               bev_ssl->ssl = NULL;
                bufferevent_free(&bev_ssl->bev.bev);
+       }
        return NULL;
 }
 
@@ -1410,15 +1415,23 @@ bufferevent_openssl_filter_new(struct event_base *base,
     int options)
 {
        BIO *bio;
+       struct bufferevent *bev;
+
        if (!underlying)
-               return NULL;
+               goto err;
        if (!(bio = BIO_new_bufferevent(underlying)))
-               return NULL;
+               goto err;
 
        SSL_set_bio(ssl, bio, bio);
 
-       return bufferevent_openssl_new_impl(
+       bev = bufferevent_openssl_new_impl(
                base, underlying, -1, ssl, state, options);
+       return bev;
+
+err:
+       if (options & BEV_OPT_CLOSE_ON_FREE)
+               SSL_free(ssl);
+       return NULL;
 }
 
 struct bufferevent *
@@ -1445,9 +1458,9 @@ bufferevent_openssl_socket_new(struct event_base *base,
                } else {
                        /* We specified an fd different from that of the SSL.
                           This is probably an error on our part.  Fail. */
-                       return NULL;
+                       goto err;
                }
-               (void) BIO_set_close(bio, 0);
+               BIO_set_close(bio, 0);
        } else {
                /* The SSL isn't configured with a BIO with an fd. */
                if (fd >= 0) {
@@ -1461,6 +1474,11 @@ bufferevent_openssl_socket_new(struct event_base *base,
 
        return bufferevent_openssl_new_impl(
                base, NULL, fd, ssl, state, options);
+
+err:
+       if (options & BEV_OPT_CLOSE_ON_FREE)
+               SSL_free(ssl);
+       return NULL;
 }
 
 int