From 981ff94d801cfca8479dce5e6fdf6f14c75f47b7 Mon Sep 17 00:00:00 2001 From: Davi Arnaut Date: Mon, 25 May 2009 10:00:18 -0300 Subject: [PATCH] Bug#42158: leak: SSL_get_peer_certificate() doesn't have matching X509_free() The problem is that the server failed to follow the rule that every X509 object retrieved using SSL_get_peer_certificate() must be explicitly freed by X509_free(). This caused a memory leak for builds linked against OpenSSL where the X509 object is reference counted -- improper counting will prevent the object from being destroyed once the session containing the peer certificate is freed. The solution is to explicitly free every X509 object used. --- mysql-test/r/openssl_1.result | 6 ++++++ mysql-test/t/openssl_1.test | 13 ++++++++++++- sql/sql_acl.cc | 11 +++++++++-- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/mysql-test/r/openssl_1.result b/mysql-test/r/openssl_1.result index c408c14b716..b0dd3acd662 100644 --- a/mysql-test/r/openssl_1.result +++ b/mysql-test/r/openssl_1.result @@ -202,4 +202,10 @@ Ssl_cipher RC4-SHA select 'is still running; no cipher request crashed the server' as result from dual; result is still running; no cipher request crashed the server +GRANT SELECT ON test.* TO bug42158@localhost REQUIRE X509; +FLUSH PRIVILEGES; +SHOW STATUS LIKE 'Ssl_cipher'; +Variable_name Value +Ssl_cipher DHE-RSA-AES256-SHA +DROP USER bug42158@localhost; End of 5.1 tests diff --git a/mysql-test/t/openssl_1.test b/mysql-test/t/openssl_1.test index 240a977fdca..baa1603faab 100644 --- a/mysql-test/t/openssl_1.test +++ b/mysql-test/t/openssl_1.test @@ -238,7 +238,18 @@ DROP TABLE t1; --enable_query_log select 'is still running; no cipher request crashed the server' as result from dual; -## +# +# Bug#42158: leak: SSL_get_peer_certificate() doesn't have matching X509_free() +# + +GRANT SELECT ON test.* TO bug42158@localhost REQUIRE X509; +FLUSH PRIVILEGES; +connect(con1,localhost,bug42158,,,,,SSL); +SHOW STATUS LIKE 'Ssl_cipher'; +disconnect con1; +connection default; +DROP USER bug42158@localhost; + --echo End of 5.1 tests # Wait till we reached the initial number of concurrent sessions diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index b1dbb7031ce..4d4e4d24684 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -936,6 +936,7 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh, #ifdef HAVE_OPENSSL Vio *vio=thd->net.vio; SSL *ssl= (SSL*) vio->ssl_arg; + X509 *cert; #endif /* @@ -964,8 +965,11 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh, */ if (vio_type(vio) == VIO_TYPE_SSL && SSL_get_verify_result(ssl) == X509_V_OK && - SSL_get_peer_certificate(ssl)) + (cert= SSL_get_peer_certificate(ssl))) + { user_access= acl_user->access; + X509_free(cert); + } break; case SSL_TYPE_SPECIFIED: /* Client should have specified attrib */ /* @@ -974,7 +978,6 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh, If cipher name is specified, we compare it to actual cipher in use. */ - X509 *cert; if (vio_type(vio) != VIO_TYPE_SSL || SSL_get_verify_result(ssl) != X509_V_OK) break; @@ -1014,6 +1017,7 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh, sql_print_information("X509 issuer mismatch: should be '%s' " "but is '%s'", acl_user->x509_issuer, ptr); free(ptr); + X509_free(cert); user_access=NO_ACCESS; break; } @@ -1033,12 +1037,15 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh, sql_print_information("X509 subject mismatch: should be '%s' but is '%s'", acl_user->x509_subject, ptr); free(ptr); + X509_free(cert); user_access=NO_ACCESS; break; } user_access= acl_user->access; free(ptr); } + /* Deallocate the X509 certificate. */ + X509_free(cert); break; #else /* HAVE_OPENSSL */ default: