vtls: Manage current easy handle in nested cfilter calls

The previous implementation cleared `data` so the outer invocation lost
its data, which could lead to a crash.

Bug: https://github.com/curl/curl/issues/10336
Reported-by: Fujii Hironori
Closes https://github.com/curl/curl/pull/10340

diff --git a/lib/cfilters.h b/lib/cfilters.h
index e101cd37e9f1ec2cd3a5879818dcf2bc0e1f6b3f..149d1ee0e37e1124e3550f0e618433397c5140df 100644 (file)
--- a/lib/cfilters.h
+++ b/lib/cfilters.h
@@ -433,4 +433,73 @@ size_t Curl_conn_get_max_concurrent(struct Curl_easy *data,
                                     struct connectdata *conn,
                                     int sockindex);
 
+
+/**
+ * Types and macros used to keep the current easy handle in filter calls,
+ * allowing for nested invocations. See #10336.
+ *
+ * `cf_call_data` is intended to be a member of the cfilter's `ctx` type.
+ * A filter defines the macro `CF_CTX_CALL_DATA` to give access to that.
+ *
+ * With all values 0, the default, this indicates that there is no cfilter
+ * call with `data` ongoing.
+ * Macro `CF_DATA_SAVE` preserves the current `cf_call_data` in a local
+ * variable and sets the `data` given, incrementing the `depth` counter.
+ *
+ * Macro `CF_DATA_RESTORE` restores the old values from the local variable,
+ * while checking that `depth` values are as expected (debug build), catching
+ * cases where a "lower" RESTORE was not called.
+ *
+ * Finally, macro `CF_DATA_CURRENT` gives the easy handle of the current
+ * invocation.
+ */
+struct cf_call_data {
+  struct Curl_easy *data;
+#ifdef DEBUGBUILD
+  int depth;
+#endif
+};
+
+/**
+ * define to access the `struct cf_call_data for a cfilter. Normally
+ * a member in the cfilter's `ctx`.
+ *
+ * #define CF_CTX_CALL_DATA(cf)   -> struct cf_call_data instance
+*/
+
+#ifdef DEBUGBUILD
+
+#define CF_DATA_SAVE(save, cf, data) \
+  do { \
+    (save) = CF_CTX_CALL_DATA(cf); \
+    DEBUGASSERT((save).data == NULL || (save).depth > 0); \
+    CF_CTX_CALL_DATA(cf).depth++;  \
+    CF_CTX_CALL_DATA(cf).data = (struct Curl_easy *)data; \
+  } while(0)
+
+#define CF_DATA_RESTORE(cf, save) \
+  do { \
+    DEBUGASSERT(CF_CTX_CALL_DATA(cf).depth == (save).depth + 1); \
+    DEBUGASSERT((save).data == NULL || (save).depth > 0); \
+    CF_CTX_CALL_DATA(cf) = (save); \
+  } while(0)
+
+#else /* DEBUGBUILD */
+
+#define CF_DATA_SAVE(save, cf, data) \
+  do { \
+    (save) = CF_CTX_CALL_DATA(cf); \
+    CF_CTX_CALL_DATA(cf).data = (struct Curl_easy *)data; \
+  } while(0)
+
+#define CF_DATA_RESTORE(cf, save) \
+  do { \
+    CF_CTX_CALL_DATA(cf) = (save); \
+  } while(0)
+
+#endif /* !DEBUGBUILD */
+
+#define CF_DATA_CURRENT(cf) \
+  ((cf)? (CF_CTX_CALL_DATA(cf).data) : NULL)
+
 #endif /* HEADER_CURL_CFILTERS_H */

diff --git a/lib/vtls/gtls.c b/lib/vtls/gtls.c
index aece7bec8317c82632e704149f26d36f69a1b098..286bce3b3660ecb16021e12ac45cc5f32bf5ceee 100644 (file)
--- a/lib/vtls/gtls.c
+++ b/lib/vtls/gtls.c
@@ -84,7 +84,7 @@ static ssize_t gtls_push(void *s, const void *buf, size_t blen)
 {
   struct Curl_cfilter *cf = s;
   struct ssl_connect_data *connssl = cf->ctx;
-  struct Curl_easy *data = connssl->call_data;
+  struct Curl_easy *data = CF_DATA_CURRENT(cf);
   ssize_t nwritten;
   CURLcode result;
 
@@ -102,7 +102,7 @@ static ssize_t gtls_pull(void *s, void *buf, size_t blen)
 {
   struct Curl_cfilter *cf = s;
   struct ssl_connect_data *connssl = cf->ctx;
-  struct Curl_easy *data = connssl->call_data;
+  struct Curl_easy *data = CF_DATA_CURRENT(cf);
   ssize_t nread;
   CURLcode result;
 

diff --git a/lib/vtls/mbedtls.c b/lib/vtls/mbedtls.c
index acb9488b6a8efb20b08ce3678070906616f2a7a3..32d0fa981b53f7b0c9eba761f340dd663beceeb1 100644 (file)
--- a/lib/vtls/mbedtls.c
+++ b/lib/vtls/mbedtls.c
@@ -159,8 +159,7 @@ static void mbed_debug(void *context, int level, const char *f_name,
 static int bio_cf_write(void *bio, const unsigned char *buf, size_t blen)
 {
   struct Curl_cfilter *cf = bio;
-  struct ssl_connect_data *connssl = cf->ctx;
-  struct Curl_easy *data = connssl->call_data;
+  struct Curl_easy *data = CF_DATA_CURRENT(cf);
   ssize_t nwritten;
   CURLcode result;
 
@@ -177,8 +176,7 @@ static int bio_cf_write(void *bio, const unsigned char *buf, size_t blen)
 static int bio_cf_read(void *bio, unsigned char *buf, size_t blen)
 {
   struct Curl_cfilter *cf = bio;
-  struct ssl_connect_data *connssl = cf->ctx;
-  struct Curl_easy *data = connssl->call_data;
+  struct Curl_easy *data = CF_DATA_CURRENT(cf);
   ssize_t nread;
   CURLcode result;
 

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index 9dbd4a4e1253a46905b246aa8b70888f192e54ad..31a8bfc4f80ba7af5d7f5007cb4e06d7a5e2605e 100644 (file)
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -702,7 +702,7 @@ static int bio_cf_out_write(BIO *bio, const char *buf, int blen)
 {
   struct Curl_cfilter *cf = BIO_get_data(bio);
   struct ssl_connect_data *connssl = cf->ctx;
-  struct Curl_easy *data = connssl->call_data;
+  struct Curl_easy *data = CF_DATA_CURRENT(cf);
   ssize_t nwritten;
   CURLcode result = CURLE_SEND_ERROR;
 
@@ -723,7 +723,7 @@ static int bio_cf_in_read(BIO *bio, char *buf, int blen)
 {
   struct Curl_cfilter *cf = BIO_get_data(bio);
   struct ssl_connect_data *connssl = cf->ctx;
-  struct Curl_easy *data = connssl->call_data;
+  struct Curl_easy *data = CF_DATA_CURRENT(cf);
   ssize_t nread;
   CURLcode result = CURLE_RECV_ERROR;
 
@@ -2639,7 +2639,6 @@ static void ossl_trace(int direction, int ssl_ver, int content_type,
   const char *verstr = "???";
   struct connectdata *conn = userp;
   int cf_idx = ossl_get_ssl_cf_index();
-  struct ssl_connect_data *connssl;
   struct Curl_easy *data = NULL;
   struct Curl_cfilter *cf;
   char unknown[32];
@@ -2647,10 +2646,7 @@ static void ossl_trace(int direction, int ssl_ver, int content_type,
   DEBUGASSERT(cf_idx >= 0);
   cf = (struct Curl_cfilter*) SSL_get_ex_data(ssl, cf_idx);
   DEBUGASSERT(cf);
-  connssl = cf->ctx;
-  DEBUGASSERT(connssl);
-  DEBUGASSERT(connssl->backend);
-  data = connssl->call_data;
+  data = CF_DATA_CURRENT(cf);
 
   if(!conn || !data || !data->set.fdebug
      || (direction != 0 && direction != 1))
@@ -2954,7 +2950,7 @@ static int ossl_new_session_cb(SSL *ssl, SSL_SESSION *ssl_sessionid)
     return 0;
   cf = (struct Curl_cfilter*) SSL_get_ex_data(ssl, cf_idx);
   connssl = cf? cf->ctx : NULL;
-  data = connssl? connssl->call_data : NULL;
+  data = connssl? CF_DATA_CURRENT(cf) : NULL;
   /* The sockindex has been stored as a pointer to an array element */
   if(!cf || !data)
     return 0;

diff --git a/lib/vtls/sectransp.c b/lib/vtls/sectransp.c
index 4897abcbd8d5b700ac35168fd645311b81cb96a9..7f311c643c492a147ef8a669e6e75a8a80e9a18f 100644 (file)
--- a/lib/vtls/sectransp.c
+++ b/lib/vtls/sectransp.c
@@ -832,7 +832,7 @@ static OSStatus bio_cf_in_read(SSLConnectionRef connection,
   struct Curl_cfilter *cf = (struct Curl_cfilter *)connection;
   struct ssl_connect_data *connssl = cf->ctx;
   struct ssl_backend_data *backend = connssl->backend;
-  struct Curl_easy *data = connssl->call_data;
+  struct Curl_easy *data = CF_DATA_CURRENT(cf);
   ssize_t nread;
   CURLcode result;
   OSStatus rtn = noErr;
@@ -868,7 +868,7 @@ static OSStatus bio_cf_out_write(SSLConnectionRef connection,
   struct Curl_cfilter *cf = (struct Curl_cfilter *)connection;
   struct ssl_connect_data *connssl = cf->ctx;
   struct ssl_backend_data *backend = connssl->backend;
-  struct Curl_easy *data = connssl->call_data;
+  struct Curl_easy *data = CF_DATA_CURRENT(cf);
   ssize_t nwritten;
   CURLcode result;
   OSStatus rtn = noErr;

diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c
index dd0414ce7c93ac8ca6dbbd63104bd111d3ab482a..f9eb4f4335f5b9992cb0658c9756ec7545a8ad28 100644 (file)
--- a/lib/vtls/vtls.c
+++ b/lib/vtls/vtls.c
@@ -315,13 +315,6 @@ static void cf_ctx_free(struct ssl_connect_data *ctx)
   }
 }
 
-static void cf_ctx_set_data(struct Curl_cfilter *cf,
-                            struct Curl_easy *data)
-{
-  if(cf->ctx)
-    ((struct ssl_connect_data *)cf->ctx)->call_data = data;
-}
-
 static CURLcode ssl_connect(struct Curl_cfilter *cf, struct Curl_easy *data)
 {
   struct ssl_connect_data *connssl = cf->ctx;
@@ -1482,8 +1475,11 @@ static CURLcode reinit_hostname(struct Curl_cfilter *cf)
 
 static void ssl_cf_destroy(struct Curl_cfilter *cf, struct Curl_easy *data)
 {
-  cf_ctx_set_data(cf, data);
+  struct cf_call_data save;
+
+  CF_DATA_SAVE(save, cf, data);
   cf_close(cf, data);
+  CF_DATA_RESTORE(cf, save);
   cf_ctx_free(cf->ctx);
   cf->ctx = NULL;
 }
@@ -1491,10 +1487,12 @@ static void ssl_cf_destroy(struct Curl_cfilter *cf, struct Curl_easy *data)
 static void ssl_cf_close(struct Curl_cfilter *cf,
                          struct Curl_easy *data)
 {
-  cf_ctx_set_data(cf, data);
+  struct cf_call_data save;
+
+  CF_DATA_SAVE(save, cf, data);
   cf_close(cf, data);
   cf->next->cft->close(cf->next, data);
-  cf_ctx_set_data(cf, NULL);
+  CF_DATA_RESTORE(cf, save);
 }
 
 static CURLcode ssl_cf_connect(struct Curl_cfilter *cf,
@@ -1502,6 +1500,7 @@ static CURLcode ssl_cf_connect(struct Curl_cfilter *cf,
                                bool blocking, bool *done)
 {
   struct ssl_connect_data *connssl = cf->ctx;
+  struct cf_call_data save;
   CURLcode result;
 
   if(cf->connected) {
@@ -1509,7 +1508,7 @@ static CURLcode ssl_cf_connect(struct Curl_cfilter *cf,
     return CURLE_OK;
   }
 
-  cf_ctx_set_data(cf, data);
+  CF_DATA_SAVE(save, cf, data);
   (void)connssl;
   DEBUGASSERT(data->conn);
   DEBUGASSERT(data->conn == cf->conn);
@@ -1540,21 +1539,22 @@ static CURLcode ssl_cf_connect(struct Curl_cfilter *cf,
     DEBUGASSERT(connssl->state == ssl_connection_complete);
   }
 out:
-  cf_ctx_set_data(cf, NULL);
+  CF_DATA_RESTORE(cf, save);
   return result;
 }
 
 static bool ssl_cf_data_pending(struct Curl_cfilter *cf,
                                 const struct Curl_easy *data)
 {
+  struct cf_call_data save;
   bool result;
 
-  cf_ctx_set_data(cf, (struct Curl_easy *)data);
+  CF_DATA_SAVE(save, cf, data);
   if(cf->ctx && Curl_ssl->data_pending(cf, data))
     result = TRUE;
   else
     result = cf->next->cft->has_data_pending(cf->next, data);
-  cf_ctx_set_data(cf, NULL);
+  CF_DATA_RESTORE(cf, save);
   return result;
 }
 
@@ -1562,12 +1562,13 @@ static ssize_t ssl_cf_send(struct Curl_cfilter *cf,
                            struct Curl_easy *data, const void *buf, size_t len,
                            CURLcode *err)
 {
+  struct cf_call_data save;
   ssize_t nwritten;
 
+  CF_DATA_SAVE(save, cf, data);
   *err = CURLE_OK;
-  cf_ctx_set_data(cf, data);
   nwritten = Curl_ssl->send_plain(cf, data, buf, len, err);
-  cf_ctx_set_data(cf, NULL);
+  CF_DATA_RESTORE(cf, save);
   return nwritten;
 }
 
@@ -1575,12 +1576,13 @@ static ssize_t ssl_cf_recv(struct Curl_cfilter *cf,
                            struct Curl_easy *data, char *buf, size_t len,
                            CURLcode *err)
 {
+  struct cf_call_data save;
   ssize_t nread;
 
+  CF_DATA_SAVE(save, cf, data);
   *err = CURLE_OK;
-  cf_ctx_set_data(cf, data);
   nread = Curl_ssl->recv_plain(cf, data, buf, len, err);
-  cf_ctx_set_data(cf, NULL);
+  CF_DATA_RESTORE(cf, save);
   return nread;
 }
 
@@ -1588,11 +1590,12 @@ static int ssl_cf_get_select_socks(struct Curl_cfilter *cf,
                                    struct Curl_easy *data,
                                    curl_socket_t *socks)
 {
+  struct cf_call_data save;
   int result;
 
-  cf_ctx_set_data(cf, data);
+  CF_DATA_SAVE(save, cf, data);
   result = Curl_ssl->get_select_socks(cf, data, socks);
-  cf_ctx_set_data(cf, NULL);
+  CF_DATA_RESTORE(cf, save);
   return result;
 }
 
@@ -1600,21 +1603,23 @@ static CURLcode ssl_cf_cntrl(struct Curl_cfilter *cf,
                              struct Curl_easy *data,
                              int event, int arg1, void *arg2)
 {
+  struct cf_call_data save;
+
   (void)arg1;
   (void)arg2;
   switch(event) {
   case CF_CTRL_DATA_ATTACH:
     if(Curl_ssl->attach_data) {
-      cf_ctx_set_data(cf, data);
+      CF_DATA_SAVE(save, cf, data);
       Curl_ssl->attach_data(cf, data);
-      cf_ctx_set_data(cf, NULL);
+      CF_DATA_RESTORE(cf, save);
     }
     break;
   case CF_CTRL_DATA_DETACH:
     if(Curl_ssl->detach_data) {
-      cf_ctx_set_data(cf, data);
+      CF_DATA_SAVE(save, cf, data);
       Curl_ssl->detach_data(cf, data);
-      cf_ctx_set_data(cf, NULL);
+      CF_DATA_RESTORE(cf, save);
     }
     break;
   default:
@@ -1625,6 +1630,8 @@ static CURLcode ssl_cf_cntrl(struct Curl_cfilter *cf,
 
 static bool cf_ssl_is_alive(struct Curl_cfilter *cf, struct Curl_easy *data)
 {
+  struct cf_call_data save;
+  bool result;
   /*
    * This function tries to determine connection status.
    *
@@ -1633,7 +1640,10 @@ static bool cf_ssl_is_alive(struct Curl_cfilter *cf, struct Curl_easy *data)
    *     0 means the connection has been closed
    *    -1 means the connection status is unknown
    */
-  return Curl_ssl->check_cxn(cf, data) != 0;
+  CF_DATA_SAVE(save, cf, data);
+  result = Curl_ssl->check_cxn(cf, data) != 0;
+  CF_DATA_RESTORE(cf, save);
+  return result;
 }
 
 struct Curl_cftype Curl_cft_ssl = {
@@ -1786,9 +1796,10 @@ void *Curl_ssl_get_internals(struct Curl_easy *data, int sockindex,
     /* get first filter in chain, if any is present */
     cf = Curl_ssl_cf_get_ssl(data->conn->cfilter[sockindex]);
     if(cf) {
-      cf_ctx_set_data(cf, data);
+      struct cf_call_data save;
+      CF_DATA_SAVE(save, cf, data);
       result = Curl_ssl->get_internals(cf->ctx, info);
-      cf_ctx_set_data(cf, NULL);
+      CF_DATA_RESTORE(cf, save);
     }
   }
   return result;

diff --git a/lib/vtls/vtls_int.h b/lib/vtls/vtls_int.h
index 1d5f415d4660a6e5078e0603e42720c9cfbf5a72..9403173af90c6cb6c254d9e005fda061746f1853 100644 (file)
--- a/lib/vtls/vtls_int.h
+++ b/lib/vtls/vtls_int.h
@@ -37,14 +37,14 @@ struct ssl_connect_data {
   char *dispname;                   /* display version of hostname */
   int port;                         /* remote port at origin */
   struct ssl_backend_data *backend; /* vtls backend specific props */
-  struct Curl_easy *call_data;      /* data handle used in current call,
-                                     * same as parameter passed, but available
-                                     * here for backend internal callbacks
-                                     * that need it. NULLed after at the
-                                     * end of each vtls filter invcocation. */
+  struct cf_call_data call_data;    /* data handle used in current call */
 };
 
 
+#define CF_CTX_CALL_DATA(cf)  \
+  ((struct ssl_connect_data *)(cf)->ctx)->call_data
+
+
 /* Definitions for SSL Implementations */
 
 struct Curl_ssl {

diff --git a/lib/vtls/wolfssl.c b/lib/vtls/wolfssl.c
index 2d78eafa087a46b15c9bd3459fc04d64a0c40bb5..93f3e2c940e400c4e3936b1eaddac4d4e8fab9f7 100644 (file)
--- a/lib/vtls/wolfssl.c
+++ b/lib/vtls/wolfssl.c
@@ -299,8 +299,7 @@ static long bio_cf_ctrl(WOLFSSL_BIO *bio, int cmd, long num, void *ptr)
 static int bio_cf_out_write(WOLFSSL_BIO *bio, const char *buf, int blen)
 {
   struct Curl_cfilter *cf = wolfSSL_BIO_get_data(bio);
-  struct ssl_connect_data *connssl = cf->ctx;
-  struct Curl_easy *data = connssl->call_data;
+  struct Curl_easy *data = CF_DATA_CURRENT(cf);
   ssize_t nwritten;
   CURLcode result = CURLE_OK;
 
@@ -315,8 +314,7 @@ static int bio_cf_out_write(WOLFSSL_BIO *bio, const char *buf, int blen)
 static int bio_cf_in_read(WOLFSSL_BIO *bio, char *buf, int blen)
 {
   struct Curl_cfilter *cf = wolfSSL_BIO_get_data(bio);
-  struct ssl_connect_data *connssl = cf->ctx;
-  struct Curl_easy *data = connssl->call_data;
+  struct Curl_easy *data = CF_DATA_CURRENT(cf);
   ssize_t nread;
   CURLcode result = CURLE_OK;