Fix a possible memory leak in ct_move_scts
authorBernd Edlinger <bernd.edlinger@hotmail.de>
Fri, 17 Nov 2023 13:47:36 +0000 (14:47 +0100)
committerTomas Mraz <tomas@openssl.org>
Tue, 28 Nov 2023 18:42:32 +0000 (19:42 +0100)
Instead of trying to move the doomed sct back
to the src stack, which may fail as well, simply
free the sct object, as the src list will be
deleted anyway.

Reviewed-by: Paul Yang <kaishen.yy@antfin.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22762)

ssl/ssl_lib.c

index 70d3b17c19e0bf28552b8e0640d1e05cdeff09af..b9858be9372958662acc745ebdd1ec4bd4ff7c42 100644 (file)
@@ -6056,6 +6056,8 @@ IMPLEMENT_OBJ_BSEARCH_GLOBAL_CMP_FN(SSL_CIPHER, SSL_CIPHER, ssl_cipher_id);
  * If |dst| points to a NULL pointer, a new stack will be created and owned by
  * the caller.
  * Returns the number of SCTs moved, or a negative integer if an error occurs.
+ * The |dst| stack is created and possibly partially populated even in case
+ * of error, likewise the |src| stack may be left in an intermediate state.
  */
 static int ct_move_scts(STACK_OF(SCT) **dst, STACK_OF(SCT) *src,
                         sct_source_t origin)
@@ -6075,15 +6077,14 @@ static int ct_move_scts(STACK_OF(SCT) **dst, STACK_OF(SCT) *src,
         if (SCT_set_source(sct, origin) != 1)
             goto err;
 
-        if (sk_SCT_push(*dst, sct) <= 0)
+        if (!sk_SCT_push(*dst, sct))
             goto err;
         scts_moved += 1;
     }
 
     return scts_moved;
  err:
-    if (sct != NULL)
-        sk_SCT_push(src, sct);  /* Put the SCT back */
+    SCT_free(sct);
     return -1;
 }