From 38f916a798c3c0592bdca296855248d5eea19a68 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Mon, 18 Jun 2018 15:50:44 -0600 Subject: [PATCH 1/6] sanity check on hashing size --- src/internal.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/internal.c b/src/internal.c index 739c730b0..574040b77 100644 --- a/src/internal.c +++ b/src/internal.c @@ -12242,6 +12242,11 @@ static INLINE int VerifyMac(WOLFSSL* ssl, const byte* input, word32 msgSz, padByte = 1; if (ssl->options.tls) { + /* Sanity check for underflow, TimingPadVerify performs hash on size + * (msgSz - ivExtra) - digestSz - pad - 1 */ + if (digestSz + pad + 1 > (msgSz - ivExtra)) { + return BUFFER_E; + } ret = TimingPadVerify(ssl, input, pad, digestSz, msgSz - ivExtra, content); if (ret != 0) From 83324f39d7a391cf6ea356401abae33562e09e5f Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 20 Jun 2018 09:10:19 -0600 Subject: [PATCH 2/6] update IO callback function names with CSharp wrapper --- wrapper/CSharp/wolfSSL_CSharp/wolfSSL.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/wrapper/CSharp/wolfSSL_CSharp/wolfSSL.cs b/wrapper/CSharp/wolfSSL_CSharp/wolfSSL.cs index 5d05a6441..96208bd6b 100644 --- a/wrapper/CSharp/wolfSSL_CSharp/wolfSSL.cs +++ b/wrapper/CSharp/wolfSSL_CSharp/wolfSSL.cs @@ -169,7 +169,7 @@ namespace wolfSSL.CSharp { [UnmanagedFunctionPointer(CallingConvention.Cdecl)] public delegate int CallbackIORecv_delegate(IntPtr ssl, IntPtr buf, int sz, IntPtr ctx); [DllImport(wolfssl_dll, CallingConvention = CallingConvention.Cdecl)] - private extern static int wolfSSL_SetIORecv(IntPtr ctx, CallbackIORecv_delegate recv); + private extern static int wolfSSL_CTX_SetIORecv(IntPtr ctx, CallbackIORecv_delegate recv); [DllImport(wolfssl_dll, CallingConvention = CallingConvention.Cdecl)] private extern static int wolfSSL_SetIOReadCtx(IntPtr ssl, IntPtr rctx); [DllImport(wolfssl_dll, CallingConvention = CallingConvention.Cdecl)] @@ -178,7 +178,7 @@ namespace wolfSSL.CSharp { [UnmanagedFunctionPointer(CallingConvention.Cdecl)] public delegate int CallbackIOSend_delegate(IntPtr ssl, IntPtr buf, int sz, IntPtr ctx); [DllImport(wolfssl_dll, CallingConvention = CallingConvention.Cdecl)] - private extern static int wolfSSL_SetIOSend(IntPtr ctx, CallbackIOSend_delegate send); + private extern static int wolfSSL_CTX_SetIOSend(IntPtr ctx, CallbackIOSend_delegate send); [DllImport(wolfssl_dll, CallingConvention = CallingConvention.Cdecl)] private extern static int wolfSSL_SetIOWriteCtx(IntPtr ssl, IntPtr wctx); [DllImport(wolfssl_dll, CallingConvention = CallingConvention.Cdecl)] @@ -825,7 +825,7 @@ namespace wolfSSL.CSharp { /* keep new function alive */ handles.set_receive(GCHandle.Alloc(func)); - wolfSSL_SetIORecv(handles.get_ctx(), func); + wolfSSL_CTX_SetIORecv(handles.get_ctx(), func); } catch (Exception e) { @@ -856,7 +856,7 @@ namespace wolfSSL.CSharp { /* keep new function alive */ handles.set_send(GCHandle.Alloc(func)); - wolfSSL_SetIOSend(handles.get_ctx(), func); + wolfSSL_CTX_SetIOSend(handles.get_ctx(), func); } catch (Exception e) { @@ -883,11 +883,11 @@ namespace wolfSSL.CSharp { CallbackIORecv_delegate recv = new CallbackIORecv_delegate(wolfssl.wolfSSLCbIORecv); io.set_receive(GCHandle.Alloc(recv)); - wolfSSL_SetIORecv(ctx, recv); + wolfSSL_CTX_SetIORecv(ctx, recv); CallbackIOSend_delegate send = new CallbackIOSend_delegate(wolfssl.wolfSSLCbIOSend); io.set_send(GCHandle.Alloc(send)); - wolfSSL_SetIOSend(ctx, send); + wolfSSL_CTX_SetIOSend(ctx, send); /* keep memory pinned */ return GCHandle.ToIntPtr(GCHandle.Alloc(io, GCHandleType.Pinned)); @@ -918,11 +918,11 @@ namespace wolfSSL.CSharp { CallbackIORecv_delegate recv = new CallbackIORecv_delegate(wolfssl.wolfSSL_dtlsCbIORecv); io.set_receive(GCHandle.Alloc(recv)); - wolfSSL_SetIORecv(ctx, recv); + wolfSSL_CTX_SetIORecv(ctx, recv); CallbackIOSend_delegate send = new CallbackIOSend_delegate(wolfssl.wolfSSL_dtlsCbIOSend); io.set_send(GCHandle.Alloc(send)); - wolfSSL_SetIOSend(ctx, send); + wolfSSL_CTX_SetIOSend(ctx, send); /* keep memory pinned */ return GCHandle.ToIntPtr(GCHandle.Alloc(io, GCHandleType.Pinned)); From 61655ef56df7d8c0a606a64523deaae4e0aa9090 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 20 Jun 2018 09:21:56 -0600 Subject: [PATCH 3/6] comment on sz value and sanity check before fuzzing --- src/internal.c | 10 ++++------ src/tls.c | 5 ++++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/internal.c b/src/internal.c index 574040b77..0b09bb592 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11991,6 +11991,9 @@ int TimingPadVerify(WOLFSSL* ssl, const byte* input, int padLen, int macSz, int ret = 0; good = MaskPadding(input, pLen, macSz); + /* 4th argument has potential to underflow, all ssl->hmac functions need to + * either increment the size by (macSz + padLen + 1) before use or check on + * the size to make sure is valid. */ ret = ssl->hmac(ssl, verify, input, pLen - macSz - padLen - 1, padLen, content, 1); good |= MaskMac(input, pLen, ssl->specs.hash_size, verify); @@ -12242,11 +12245,6 @@ static INLINE int VerifyMac(WOLFSSL* ssl, const byte* input, word32 msgSz, padByte = 1; if (ssl->options.tls) { - /* Sanity check for underflow, TimingPadVerify performs hash on size - * (msgSz - ivExtra) - digestSz - pad - 1 */ - if (digestSz + pad + 1 > (msgSz - ivExtra)) { - return BUFFER_E; - } ret = TimingPadVerify(ssl, input, pad, digestSz, msgSz - ivExtra, content); if (ret != 0) @@ -13034,7 +13032,7 @@ static int SSL_hmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, (void)padLen; #ifdef HAVE_FUZZER - if (ssl->fuzzerCb) + if (ssl->fuzzerCb && (int)sz > 0) ssl->fuzzerCb(ssl, in, sz, FUZZ_HMAC, ssl->fuzzerCtx); #endif diff --git a/src/tls.c b/src/tls.c index b19f4b894..8d8e52e81 100644 --- a/src/tls.c +++ b/src/tls.c @@ -1300,7 +1300,10 @@ int TLS_hmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, int padSz, return BAD_FUNC_ARG; #ifdef HAVE_FUZZER - if (ssl->fuzzerCb) + /* sz argument has potential to underflow, all ssl->hmac functions need to + * either increment the size by (macSz + padLen + 1) before use or check on + * the size to make sure is valid when sz is effected by IO */ + if (ssl->fuzzerCb && (int)sz > 0) ssl->fuzzerCb(ssl, in, sz, FUZZ_HMAC, ssl->fuzzerCtx); #endif From 777c89a2571089cb4684a5752b4f2adcb4f94834 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 20 Jun 2018 09:37:36 -0600 Subject: [PATCH 4/6] sanity check on pointer --- wolfcrypt/src/asn.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 500296088..65b479f43 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -7508,13 +7508,17 @@ static int wc_EncryptedInfoParse(EncryptedInfo* info, if (start == NULL) return BUFFER_E; - if (start >= bufferEnd) - return BUFFER_E; /* skip dec-info and ": " */ start += XSTRLEN(kDecInfoHeader); - if (start[0] == ':') + if (start >= bufferEnd) + return BUFFER_E; + + if (start[0] == ':') { start++; + if (start >= bufferEnd) + return BUFFER_E; + } if (start[0] == ' ') start++; From 2f43d5eecea6ce858ca2f7d6c789e0122606d38e Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 20 Jun 2018 15:29:05 -0600 Subject: [PATCH 5/6] update size to be used with fuzzing --- src/internal.c | 4 ++-- src/tls.c | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/internal.c b/src/internal.c index 0b09bb592..dfb3a2fe9 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11991,7 +11991,7 @@ int TimingPadVerify(WOLFSSL* ssl, const byte* input, int padLen, int macSz, int ret = 0; good = MaskPadding(input, pLen, macSz); - /* 4th argument has potential to underflow, all ssl->hmac functions need to + /* 4th argument has potential to underflow, ssl->hmac function should * either increment the size by (macSz + padLen + 1) before use or check on * the size to make sure is valid. */ ret = ssl->hmac(ssl, verify, input, pLen - macSz - padLen - 1, padLen, @@ -13032,7 +13032,7 @@ static int SSL_hmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, (void)padLen; #ifdef HAVE_FUZZER - if (ssl->fuzzerCb && (int)sz > 0) + if (ssl->fuzzerCb) ssl->fuzzerCb(ssl, in, sz, FUZZ_HMAC, ssl->fuzzerCtx); #endif diff --git a/src/tls.c b/src/tls.c index 8d8e52e81..9f0c49497 100644 --- a/src/tls.c +++ b/src/tls.c @@ -1300,11 +1300,16 @@ int TLS_hmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, int padSz, return BAD_FUNC_ARG; #ifdef HAVE_FUZZER - /* sz argument has potential to underflow, all ssl->hmac functions need to - * either increment the size by (macSz + padLen + 1) before use or check on - * the size to make sure is valid when sz is effected by IO */ - if (ssl->fuzzerCb && (int)sz > 0) - ssl->fuzzerCb(ssl, in, sz, FUZZ_HMAC, ssl->fuzzerCtx); + /* Fuzz "in" buffer with sz to be used in HMAC algorithm */ + if (ssl->fuzzerCb) { + if (verify && padSz >= 0) { + ssl->fuzzerCb(ssl, in, sz + ssl->specs.hash_size + padSz + 1, + FUZZ_HMAC, ssl->fuzzerCtx); + } + else { + ssl->fuzzerCb(ssl, in, sz, FUZZ_HMAC, ssl->fuzzerCtx); + } + } #endif wolfSSL_SetTlsHmacInner(ssl, myInner, sz, content, verify); From bf6300323739a6a1fe5a643bc3dd19889b1f7113 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 20 Jun 2018 16:48:40 -0600 Subject: [PATCH 6/6] sanity check before reading word16 from buffer --- src/tls13.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tls13.c b/src/tls13.c index cb30d0536..ac9bd57eb 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -2866,6 +2866,8 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #endif { /* Get extension length and length check. */ + if ((i - begin) + OPAQUE16_LEN > helloSz) + return BUFFER_ERROR; ato16(&input[i], &totalExtSz); i += OPAQUE16_LEN; if ((i - begin) + totalExtSz > helloSz)