From 422683f4c31ef6e0b159340fc6db5a7e8a884d2c Mon Sep 17 00:00:00 2001
From: John Safranek <john@wolfssl.com>
Date: Tue, 13 Oct 2020 10:15:58 -0700
Subject: [PATCH] Fuzz Fix GetPublicDhKey() assumes the ssl session owns the DH
 public key parts, and tries to free them. They belong to the CTX initially,
 so it shouldn't be freeing them, necessarily.

1. Add a check for weOwnDh first, then free the buffers if needed.
2. If there is a problem reading the keys, free the new buffers before exiting.
3. Set weOwnDh once the buffers and values have been stored
   successfully.
---
 src/internal.c | 57 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 44 insertions(+), 13 deletions(-)

diff --git a/src/internal.c b/src/internal.c
index 9122c43d2..2b52424ac 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -21133,21 +21133,24 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
     int             group = 0;
 #endif
 
-    ssl->buffers.weOwnDH = 1;
+    if (ssl->buffers.weOwnDH) {
+        if (ssl->buffers.serverDH_P.buffer) {
+            XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap,
+                    DYNAMIC_TYPE_PUBLIC_KEY);
+            ssl->buffers.serverDH_P.buffer = NULL;
+        }
 
-    if (ssl->buffers.serverDH_P.buffer) {
-        XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY);
-        ssl->buffers.serverDH_P.buffer = NULL;
-    }
+        if (ssl->buffers.serverDH_G.buffer) {
+            XFREE(ssl->buffers.serverDH_G.buffer, ssl->heap,
+                    DYNAMIC_TYPE_PUBLIC_KEY);
+            ssl->buffers.serverDH_G.buffer = NULL;
+        }
 
-    if (ssl->buffers.serverDH_G.buffer) {
-        XFREE(ssl->buffers.serverDH_G.buffer, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY);
-        ssl->buffers.serverDH_G.buffer = NULL;
-    }
-
-    if (ssl->buffers.serverDH_Pub.buffer) {
-        XFREE(ssl->buffers.serverDH_Pub.buffer, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY);
-        ssl->buffers.serverDH_Pub.buffer = NULL;
+        if (ssl->buffers.serverDH_Pub.buffer) {
+            XFREE(ssl->buffers.serverDH_Pub.buffer, ssl->heap,
+                    DYNAMIC_TYPE_PUBLIC_KEY);
+            ssl->buffers.serverDH_Pub.buffer = NULL;
+        }
     }
 
     /* p */
@@ -21190,6 +21193,9 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
 
     /* g */
     if ((args->idx - args->begin) + OPAQUE16_LEN > size) {
+        XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap,
+                DYNAMIC_TYPE_PUBLIC_KEY);
+        ssl->buffers.serverDH_P.buffer = NULL;
         ERROR_OUT(BUFFER_ERROR, exit_gdpk);
     }
 
@@ -21197,6 +21203,9 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
     args->idx += OPAQUE16_LEN;
 
     if ((args->idx - args->begin) + length > size) {
+        XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap,
+                DYNAMIC_TYPE_PUBLIC_KEY);
+        ssl->buffers.serverDH_P.buffer = NULL;
         ERROR_OUT(BUFFER_ERROR, exit_gdpk);
     }
 
@@ -21206,6 +21215,9 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
         ssl->buffers.serverDH_G.length = length;
     }
     else {
+        XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap,
+                DYNAMIC_TYPE_PUBLIC_KEY);
+        ssl->buffers.serverDH_P.buffer = NULL;
         ERROR_OUT(MEMORY_ERROR, exit_gdpk);
     }
 
@@ -21215,6 +21227,12 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
 
     /* pub */
     if ((args->idx - args->begin) + OPAQUE16_LEN > size) {
+        XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap,
+                DYNAMIC_TYPE_PUBLIC_KEY);
+        ssl->buffers.serverDH_P.buffer = NULL;
+        XFREE(ssl->buffers.serverDH_G.buffer, ssl->heap,
+                DYNAMIC_TYPE_PUBLIC_KEY);
+        ssl->buffers.serverDH_G.buffer = NULL;
         ERROR_OUT(BUFFER_ERROR, exit_gdpk);
     }
 
@@ -21222,6 +21240,12 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
     args->idx += OPAQUE16_LEN;
 
     if ((args->idx - args->begin) + length > size) {
+        XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap,
+                DYNAMIC_TYPE_PUBLIC_KEY);
+        ssl->buffers.serverDH_P.buffer = NULL;
+        XFREE(ssl->buffers.serverDH_G.buffer, ssl->heap,
+                DYNAMIC_TYPE_PUBLIC_KEY);
+        ssl->buffers.serverDH_G.buffer = NULL;
         ERROR_OUT(BUFFER_ERROR, exit_gdpk);
     }
 
@@ -21231,11 +21255,18 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
         ssl->buffers.serverDH_Pub.length = length;
     }
     else {
+        XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap,
+                DYNAMIC_TYPE_PUBLIC_KEY);
+        ssl->buffers.serverDH_P.buffer = NULL;
+        XFREE(ssl->buffers.serverDH_G.buffer, ssl->heap,
+                DYNAMIC_TYPE_PUBLIC_KEY);
+        ssl->buffers.serverDH_G.buffer = NULL;
         ERROR_OUT(MEMORY_ERROR, exit_gdpk);
     }
 
     XMEMCPY(ssl->buffers.serverDH_Pub.buffer, input + args->idx,
                                                         length);
+    ssl->buffers.weOwnDH = 1;
     args->idx += length;
 
 #ifdef HAVE_FFDHE