Fix memory leak when freeing the DTLS record layer

We need to check whether the sent_messages has actually buffered any
messages in it. If not we won't free the old record layer later when we
clear out the old buffered messages and a memory leak will result.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19586)

diff --git a/ssl/record/methods/dtls_meth.c b/ssl/record/methods/dtls_meth.c
index 7cd3d51976da172683a96db99950bcd914c9b9dd..858417d965f3400ec2c28b5a38f6b9df0fb3a6b1 100644 (file)
--- a/ssl/record/methods/dtls_meth.c
+++ b/ssl/record/methods/dtls_meth.c
@@ -684,7 +684,7 @@ dtls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,
 
  err:
     if (ret != OSSL_RECORD_RETURN_SUCCESS) {
-        OPENSSL_free(*retrl);
+        dtls_free(*retrl);
         *retrl = NULL;
     }
     return ret;

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 9a4cd853894abac46bcb9d566f27f71f737ee3e6..dc0b8b3d9efe4e11424ee45697607bf057aae787 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1356,11 +1356,14 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
 
     /*
      * Free the old record layer if we have one except in the case of DTLS when
-     * writing. In that case the record layer is still referenced by buffered
-     * messages for potential retransmit. Only when those buffered messages get
-     * freed do we free the record layer object (see dtls1_hm_fragment_free)
+     * writing and there are still buffered sent messages in our queue. In that
+     * case the record layer is still referenced by those buffered messages for
+     * potential retransmit. Only when those buffered messages get freed do we
+     * free the record layer object (see dtls1_hm_fragment_free)
      */
-    if (!SSL_CONNECTION_IS_DTLS(s) || direction == OSSL_RECORD_DIRECTION_READ) {
+    if (!SSL_CONNECTION_IS_DTLS(s)
+            || direction == OSSL_RECORD_DIRECTION_READ
+            || pqueue_peek(s->d1->sent_messages) == NULL) {
         if (*thismethod != NULL && !(*thismethod)->free(*thisrl)) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
             return 0;