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.
This commit is contained in:
John Safranek
2020-10-13 10:15:58 -07:00
parent 724eb96047
commit 422683f4c3

View File

@@ -21133,22 +21133,25 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
int group = 0; int group = 0;
#endif #endif
ssl->buffers.weOwnDH = 1; if (ssl->buffers.weOwnDH) {
if (ssl->buffers.serverDH_P.buffer) { if (ssl->buffers.serverDH_P.buffer) {
XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); XFREE(ssl->buffers.serverDH_P.buffer, ssl->heap,
DYNAMIC_TYPE_PUBLIC_KEY);
ssl->buffers.serverDH_P.buffer = NULL; ssl->buffers.serverDH_P.buffer = NULL;
} }
if (ssl->buffers.serverDH_G.buffer) { if (ssl->buffers.serverDH_G.buffer) {
XFREE(ssl->buffers.serverDH_G.buffer, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); XFREE(ssl->buffers.serverDH_G.buffer, ssl->heap,
DYNAMIC_TYPE_PUBLIC_KEY);
ssl->buffers.serverDH_G.buffer = NULL; ssl->buffers.serverDH_G.buffer = NULL;
} }
if (ssl->buffers.serverDH_Pub.buffer) { if (ssl->buffers.serverDH_Pub.buffer) {
XFREE(ssl->buffers.serverDH_Pub.buffer, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); XFREE(ssl->buffers.serverDH_Pub.buffer, ssl->heap,
DYNAMIC_TYPE_PUBLIC_KEY);
ssl->buffers.serverDH_Pub.buffer = NULL; ssl->buffers.serverDH_Pub.buffer = NULL;
} }
}
/* p */ /* p */
if ((args->idx - args->begin) + OPAQUE16_LEN > size) { if ((args->idx - args->begin) + OPAQUE16_LEN > size) {
@@ -21190,6 +21193,9 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
/* g */ /* g */
if ((args->idx - args->begin) + OPAQUE16_LEN > size) { 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); ERROR_OUT(BUFFER_ERROR, exit_gdpk);
} }
@@ -21197,6 +21203,9 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
args->idx += OPAQUE16_LEN; args->idx += OPAQUE16_LEN;
if ((args->idx - args->begin) + length > size) { 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); 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; ssl->buffers.serverDH_G.length = length;
} }
else { 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); ERROR_OUT(MEMORY_ERROR, exit_gdpk);
} }
@@ -21215,6 +21227,12 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
/* pub */ /* pub */
if ((args->idx - args->begin) + OPAQUE16_LEN > size) { 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); ERROR_OUT(BUFFER_ERROR, exit_gdpk);
} }
@@ -21222,6 +21240,12 @@ static int GetDhPublicKey(WOLFSSL* ssl, const byte* input, word32 size,
args->idx += OPAQUE16_LEN; args->idx += OPAQUE16_LEN;
if ((args->idx - args->begin) + length > size) { 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); 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; ssl->buffers.serverDH_Pub.length = length;
} }
else { 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); ERROR_OUT(MEMORY_ERROR, exit_gdpk);
} }
XMEMCPY(ssl->buffers.serverDH_Pub.buffer, input + args->idx, XMEMCPY(ssl->buffers.serverDH_Pub.buffer, input + args->idx,
length); length);
ssl->buffers.weOwnDH = 1;
args->idx += length; args->idx += length;
#ifdef HAVE_FFDHE #ifdef HAVE_FFDHE