From 1c461009c2b82dda58f2e39257db7dad8aff41b0 Mon Sep 17 00:00:00 2001 From: Michael Zillgith Date: Wed, 23 Jan 2019 23:54:15 +0100 Subject: [PATCH] - MMS client/server: fixed potential memory leaks in TLS handling code --- examples/tls_client_example/tls_client_example.c | 10 +++++++++- hal/tls/mbedtls/tls_mbedtls.c | 5 ++++- src/mms/iso_client/iso_client_connection.c | 2 ++ src/mms/iso_mms/client/mms_client_connection.c | 2 -- src/mms/iso_server/iso_connection.c | 10 +++++++++- 5 files changed, 24 insertions(+), 5 deletions(-) diff --git a/examples/tls_client_example/tls_client_example.c b/examples/tls_client_example/tls_client_example.c index 77e2b994..a2a774d8 100644 --- a/examples/tls_client_example/tls_client_example.c +++ b/examples/tls_client_example/tls_client_example.c @@ -66,7 +66,13 @@ int main(int argc, char** argv) { if (error == IED_ERROR_OK) { - IedConnection_getServerDirectory(con, &error, false); + LinkedList serverDirectory = IedConnection_getServerDirectory(con, &error, false); + + if (error != IED_ERROR_OK) + printf("failed to read server directory (error=%i)\n", error); + + if (serverDirectory) + LinkedList_destroy(serverDirectory); /* read an analog measurement value from server */ MmsValue* value = IedConnection_readObject(con, &error, "simpleIOGenericIO/GGIO1.AnIn1.mag.f", IEC61850_FC_MX); @@ -146,6 +152,8 @@ int main(int argc, char** argv) { } IedConnection_destroy(con); + + TLSConfiguration_destroy(tlsConfig); } diff --git a/hal/tls/mbedtls/tls_mbedtls.c b/hal/tls/mbedtls/tls_mbedtls.c index 4b035ec2..f5b7f6f8 100644 --- a/hal/tls/mbedtls/tls_mbedtls.c +++ b/hal/tls/mbedtls/tls_mbedtls.c @@ -476,7 +476,7 @@ TLSSocket_close(TLSSocket self) { int ret; - //TODO add timeout? + /* TODO add timeout? */ while ((ret = mbedtls_ssl_close_notify(&(self->ssl))) < 0) { @@ -492,5 +492,8 @@ TLSSocket_close(TLSSocket self) mbedtls_ssl_config_free(&(self->conf)); mbedtls_ssl_free(&(self->ssl)); + if (self->peerCert) + GLOBAL_FREEMEM(self->peerCert); + GLOBAL_FREEMEM(self); } diff --git a/src/mms/iso_client/iso_client_connection.c b/src/mms/iso_client/iso_client_connection.c index 74f0108a..e8f6aab4 100644 --- a/src/mms/iso_client/iso_client_connection.c +++ b/src/mms/iso_client/iso_client_connection.c @@ -201,6 +201,8 @@ sendConnectionRequestMessage(IsoClientConnection self) #if (CONFIG_MMS_SUPPORT_TLS == 1) if (self->parameters->tlsConfiguration) { + TLSConfiguration_setClientMode(self->parameters->tlsConfiguration); + /* create TLSSocket and start TLS authentication */ TLSSocket tlsSocket = TLSSocket_create(self->socket, self->parameters->tlsConfiguration, false); diff --git a/src/mms/iso_mms/client/mms_client_connection.c b/src/mms/iso_mms/client/mms_client_connection.c index af32fc5d..f27a55a1 100644 --- a/src/mms/iso_mms/client/mms_client_connection.c +++ b/src/mms/iso_mms/client/mms_client_connection.c @@ -1386,8 +1386,6 @@ MmsConnection_createInternal(TLSConfiguration tlsConfig, bool createThread) #if (CONFIG_MMS_SUPPORT_TLS == 1) if (tlsConfig) { - TLSConfiguration_setClientMode(tlsConfig); - IsoConnectionParameters_setTlsConfiguration(self->isoParameters, tlsConfig); } #endif /* (CONFIG_MMS_SUPPORT_TLS == 1) */ diff --git a/src/mms/iso_server/iso_connection.c b/src/mms/iso_server/iso_connection.c index a7705a25..6ad6c7e9 100644 --- a/src/mms/iso_server/iso_connection.c +++ b/src/mms/iso_server/iso_connection.c @@ -96,6 +96,12 @@ finalizeIsoConnection(IsoConnection self) printf("ISO_SERVER: finalizeIsoConnection --> close transport connection\n"); IsoServer_closeConnection(self->isoServer, self); + +#if (CONFIG_MMS_SUPPORT_TLS == 1) + if (self->tlsSocket) + TLSSocket_close(self->tlsSocket); +#endif + if (self->socket != NULL) Socket_destroy(self->socket); @@ -601,8 +607,10 @@ IsoConnection_close(IsoConnection self) self->socket = NULL; #if (CONFIG_MMS_SUPPORT_TLS == 1) - if (self->tlsSocket) + if (self->tlsSocket) { TLSSocket_close(self->tlsSocket); + self->tlsSocket = NULL; + } #endif Socket_destroy(socket);