mod_ssl uses free() inappropriately in several places, to free

trawick · trawick · commit 3c8a9a6ce8a4 · 2002-10-25T21:44:28.000Z
memory which has been previously allocated inside OpenSSL. Such memory should be freed with OPENSSL_free(), not with free(). Submitted by: Nadav Har'El <nyh@math.technion.ac.il>, Madhusudan Mathihalli <madhusudan_mathihalli@hp.com> Reviewed by: Jeff Trawick git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@97307 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/CHANGES b/CHANGES
@@ -1,5 +1,11 @@
 Changes with Apache 2.0.44
 
+  *) mod_ssl uses free() inappropriately in several places, to free
+     memory which has been previously allocated inside OpenSSL.
+     Such memory should be freed with OPENSSL_free(), not with free().
+     [Nadav Har'El <nyh@math.technion.ac.il>,
+      Madhusudan Mathihalli <madhusudan_mathihalli@hp.com>].
+
   *) Emit a message to the error log when we return 404 because
      the URI contained '%2f'.  (This was previously nastily silent
      and difficult to debug.)  [Ken Coar]
diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c
@@ -968,7 +968,7 @@ int ssl_hook_UserCheck(request_rec *r)
         X509_NAME *name = X509_get_subject_name(sslconn->client_cert);
         char *cp = X509_NAME_oneline(name, NULL, 0);
         sslconn->client_dn = apr_pstrdup(r->connection->pool, cp);
-        free(cp);
+        modssl_free(cp);
     }
 
     clientdn = (char *)sslconn->client_dn;
@@ -1299,11 +1299,11 @@ int ssl_callback_SSLVerify(int ok, X509_STORE_CTX *ctx)
                      iname ? iname : "-unknown-");
 
         if (sname) {
-            free(sname);
+            modssl_free(sname);
         }
 
         if (iname) {
-            free(iname);
+            modssl_free(iname);
         }
     }
 
@@ -1555,7 +1555,7 @@ int ssl_callback_SSLVerify_CRL(int ok, X509_STORE_CTX *ctx, conn_rec *c)
                                  "Certificate with serial %ld (0x%lX) "
                                  "revoked per CRL from issuer %s",
                                  serial, serial, cp);
-                    free(cp);
+                    modssl_free(cp);
                 }
 
                 X509_STORE_CTX_set_error(ctx, X509_V_ERR_CERT_REVOKED);
@@ -1593,6 +1593,7 @@ static void modssl_proxy_info_log(server_rec *s,
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                  SSLPROXY_CERT_CB_LOG_FMT "%s, sending %s", 
                  sc->vhost_id, msg, dn ? dn : "-uknown-");
+    modssl_free(dn);
 }
 
 /*
diff --git a/modules/ssl/ssl_engine_vars.c b/modules/ssl/ssl_engine_vars.c
@@ -334,7 +334,7 @@ static char *ssl_var_lookup_ssl_cert(apr_pool_t *p, X509 *xs, char *var)
         xsname = X509_get_subject_name(xs);
         cp = X509_NAME_oneline(xsname, NULL, 0);
         result = apr_pstrdup(p, cp);
-        free(cp);
+        modssl_free(cp);
         resdup = FALSE;
     }
     else if (strlen(var) > 5 && strcEQn(var, "S_DN_", 5)) {
@@ -346,7 +346,7 @@ static char *ssl_var_lookup_ssl_cert(apr_pool_t *p, X509 *xs, char *var)
         xsname = X509_get_issuer_name(xs);
         cp = X509_NAME_oneline(xsname, NULL, 0);
         result = apr_pstrdup(p, cp);
-        free(cp);
+        modssl_free(cp);
         resdup = FALSE;
     }
     else if (strlen(var) > 5 && strcEQn(var, "I_DN_", 5)) {
diff --git a/modules/ssl/ssl_toolkit_compat.h b/modules/ssl/ssl_toolkit_compat.h
@@ -105,6 +105,8 @@
 
 #define modssl_set_cipher_list SSL_set_cipher_list
 
+#define modssl_free OPENSSL_free
+
 #define EVP_PKEY_reference_inc(pkey) \
    CRYPTO_add(&((pkey)->references), +1, CRYPTO_LOCK_X509_PKEY)
 
@@ -148,6 +150,8 @@
 #define modssl_set_cipher_list(ssl, l) \
    SSL_set_cipher_list(ssl, (char *)l)
 
+#define modssl_free free
+
 #ifndef PEM_F_DEF_CALLBACK
 #define PEM_F_DEF_CALLBACK PEM_F_DEF_CB
 #endif

Original file line number	Diff line number	Diff line change
`@@ -968,7 +968,7 @@ int ssl_hook_UserCheck(request_rec *r)`
`968`	`968`	`X509_NAME *name = X509_get_subject_name(sslconn->client_cert);`
`969`	`969`	`char *cp = X509_NAME_oneline(name, NULL, 0);`
`970`	`970`	`sslconn->client_dn = apr_pstrdup(r->connection->pool, cp);`
`971`		`- free(cp);`
	`971`	`+ modssl_free(cp);`
`972`	`972`	`}`
`973`	`973`
`974`	`974`	`clientdn = (char *)sslconn->client_dn;`
`@@ -1299,11 +1299,11 @@ int ssl_callback_SSLVerify(int ok, X509_STORE_CTX *ctx)`
`1299`	`1299`	`iname ? iname : "-unknown-");`
`1300`	`1300`
`1301`	`1301`	`if (sname) {`
`1302`		`- free(sname);`
	`1302`	`+ modssl_free(sname);`
`1303`	`1303`	`}`
`1304`	`1304`
`1305`	`1305`	`if (iname) {`
`1306`		`- free(iname);`
	`1306`	`+ modssl_free(iname);`
`1307`	`1307`	`}`
`1308`	`1308`	`}`
`1309`	`1309`
`@@ -1555,7 +1555,7 @@ int ssl_callback_SSLVerify_CRL(int ok, X509_STORE_CTX ctx, conn_rec c)`
`1555`	`1555`	`"Certificate with serial %ld (0x%lX) "`
`1556`	`1556`	`"revoked per CRL from issuer %s",`
`1557`	`1557`	`serial, serial, cp);`
`1558`		`- free(cp);`
	`1558`	`+ modssl_free(cp);`
`1559`	`1559`	`}`
`1560`	`1560`
`1561`	`1561`	`X509_STORE_CTX_set_error(ctx, X509_V_ERR_CERT_REVOKED);`
`@@ -1593,6 +1593,7 @@ static void modssl_proxy_info_log(server_rec *s,`
`1593`	`1593`	`ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,`
`1594`	`1594`	`SSLPROXY_CERT_CB_LOG_FMT "%s, sending %s",`
`1595`	`1595`	`sc->vhost_id, msg, dn ? dn : "-uknown-");`
	`1596`	`+ modssl_free(dn);`
`1596`	`1597`	`}`
`1597`	`1598`
`1598`	`1599`	`/*`