Attachment 17561 Details for Bug 15134 – Patch for Heimdal master (v4)

[patch] Patch for Heimdal master (v4)

CVE-2022-3437-des3-overflow-v4-for-heimdal.patch (text/plain), 13.64 KB, created by Jo Sutton on 2022-10-13 06:23:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jo Sutton

Created: 2022-10-13 06:23:25 UTC

Size: 13.64 KB

patch

obsolete

>From 504cf509ea07a130bfd2540804e70cdf0e84b219 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Wed, 12 Oct 2022 13:56:08 +1300
>Subject: [PATCH 1/9] CVE-2022-3437 lib/krb5: Remove __func__ compatibility
> workaround
>
>As described by the C standard, __func__ is a variable, not a macro.
>Hence this #ifndef check does not work as intended, and only serves to
>unconditionally disable __func__. A nonoperating __func__ prevents
>cmocka operating correctly, so remove this definition.
>
>Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/krb5/krb5_locl.h | 4 ----
> 1 file changed, 4 deletions(-)
>
>diff --git a/lib/krb5/krb5_locl.h b/lib/krb5/krb5_locl.h
>index 6b74653f56..666e6549b5 100644
>--- a/lib/krb5/krb5_locl.h
>+++ b/lib/krb5/krb5_locl.h
>@@ -202,10 +202,6 @@ extern const char _krb5_wellknown_lkdc[];
> #define ALLOC(X, N) (X) = calloc((N), sizeof(*(X)))
> #define ALLOC_SEQ(X, N) do { (X)->len = (N); ALLOC((X)->val, (N)); } while(0)
> 
>-#ifndef __func__
>-#define __func__ "unknown-function"
>-#endif
>-
> #define krb5_einval(context, argnum) _krb5_einval((context), __func__, (argnum))
> 
> #ifndef PATH_SEP
>-- 
>2.35.0
>
>
>From cd558fd2fbe46461115f67966a3995f8b7172e96 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Wed, 12 Oct 2022 13:57:13 +1300
>Subject: [PATCH 2/9] CVE-2022-3437 lib/gssapi/krb5: Use constant-time memcmp()
> for arcfour unwrap
>
>Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/gssapi/krb5/arcfour.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/lib/gssapi/krb5/arcfour.c b/lib/gssapi/krb5/arcfour.c
>index d52a7d33d2..5c754bc6d5 100644
>--- a/lib/gssapi/krb5/arcfour.c
>+++ b/lib/gssapi/krb5/arcfour.c
>@@ -388,9 +388,9 @@ _gssapi_verify_mic_arcfour(OM_uint32 * minor_status,
>     _gsskrb5_decode_be_om_uint32(SND_SEQ, &seq_number);
> 
>     if (context_handle->more_flags & LOCAL)
>-	cmp = (memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4) != 0);
>+	cmp = (ct_memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4) != 0);
>     else
>-	cmp = (memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0);
>+	cmp = (ct_memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0);
> 
>     memset_s(SND_SEQ, sizeof(SND_SEQ), 0, sizeof(SND_SEQ));
>     if (cmp != 0) {
>@@ -659,9 +659,9 @@ OM_uint32 _gssapi_unwrap_arcfour(OM_uint32 *minor_status,
>     _gsskrb5_decode_be_om_uint32(SND_SEQ, &seq_number);
> 
>     if (context_handle->more_flags & LOCAL)
>-	cmp = (memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4) != 0);
>+	cmp = (ct_memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4) != 0);
>     else
>-	cmp = (memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0);
>+	cmp = (ct_memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0);
> 
>     if (cmp != 0) {
> 	*minor_status = 0;
>@@ -1282,9 +1282,9 @@ _gssapi_unwrap_iov_arcfour(OM_uint32 *minor_status,
>     _gsskrb5_decode_be_om_uint32(snd_seq, &seq_number);
> 
>     if (ctx->more_flags & LOCAL) {
>-	cmp = (memcmp(&snd_seq[4], "\xff\xff\xff\xff", 4) != 0);
>+	cmp = (ct_memcmp(&snd_seq[4], "\xff\xff\xff\xff", 4) != 0);
>     } else {
>-	cmp = (memcmp(&snd_seq[4], "\x00\x00\x00\x00", 4) != 0);
>+	cmp = (ct_memcmp(&snd_seq[4], "\x00\x00\x00\x00", 4) != 0);
>     }
>     if (cmp != 0) {
> 	*minor_status = 0;
>-- 
>2.35.0
>
>
>From 94f2bd5fdda3cba1b850ac800893b379d6b2c054 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Wed, 12 Oct 2022 13:57:55 +1300
>Subject: [PATCH 3/9] CVE-2022-3437 lib/gssapi/krb5: Use constant-time memcmp()
> in unwrap_des3()
>
>The surrounding checks all use ct_memcmp(), so this one was presumably
>meant to as well.
>
>Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/gssapi/krb5/unwrap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/gssapi/krb5/unwrap.c b/lib/gssapi/krb5/unwrap.c
>index f37b0a653e..e36491b6f9 100644
>--- a/lib/gssapi/krb5/unwrap.c
>+++ b/lib/gssapi/krb5/unwrap.c
>@@ -230,7 +230,7 @@ unwrap_des3
>   if (ret)
>       return ret;
> 
>-  if (memcmp (p, "\x04\x00", 2) != 0) /* HMAC SHA1 DES3_KD */
>+  if (ct_memcmp (p, "\x04\x00", 2) != 0) /* HMAC SHA1 DES3_KD */
>     return GSS_S_BAD_SIG;
>   p += 2;
>   if (ct_memcmp (p, "\x02\x00", 2) == 0) {
>-- 
>2.35.0
>
>
>From 1b1876f7969c18a3d65f63012ad08b8624a3ac6e Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Wed, 12 Oct 2022 13:57:42 +1300
>Subject: [PATCH 4/9] CVE-2022-3437 lib/gssapi/krb5: Don't pass NULL pointers
> to memcpy() in DES unwrap
>
>Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/gssapi/krb5/unwrap.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
>diff --git a/lib/gssapi/krb5/unwrap.c b/lib/gssapi/krb5/unwrap.c
>index e36491b6f9..61ca29156a 100644
>--- a/lib/gssapi/krb5/unwrap.c
>+++ b/lib/gssapi/krb5/unwrap.c
>@@ -183,9 +183,10 @@ unwrap_des
>   output_message_buffer->value  = malloc(output_message_buffer->length);
>   if(output_message_buffer->length != 0 && output_message_buffer->value == NULL)
>       return GSS_S_FAILURE;
>-  memcpy (output_message_buffer->value,
>-	  p + 24,
>-	  output_message_buffer->length);
>+  if (output_message_buffer->value != NULL)
>+      memcpy (output_message_buffer->value,
>+	      p + 24,
>+	      output_message_buffer->length);
>   return GSS_S_COMPLETE;
> }
> #endif
>@@ -377,9 +378,10 @@ unwrap_des3
>   output_message_buffer->value  = malloc(output_message_buffer->length);
>   if(output_message_buffer->length != 0 && output_message_buffer->value == NULL)
>       return GSS_S_FAILURE;
>-  memcpy (output_message_buffer->value,
>-	  p + 36,
>-	  output_message_buffer->length);
>+  if (output_message_buffer->value != NULL)
>+      memcpy (output_message_buffer->value,
>+	      p + 36,
>+	      output_message_buffer->length);
>   return GSS_S_COMPLETE;
> }
> 
>-- 
>2.35.0
>
>
>From 7983b181d0d2d6675c5cc8d4c7b9ef971b987369 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Mon, 15 Aug 2022 16:53:45 +1200
>Subject: [PATCH 5/9] CVE-2022-3437 lib/gssapi/krb5: Avoid undefined behaviour
> in _gssapi_verify_pad()
>
>By decrementing 'pad' only when we know it's safe, we ensure we can't
>stray backwards past the start of a buffer, which would be undefined
>behaviour.
>
>In the previous version of the loop, 'i' is the number of bytes left to
>check, and 'pad' is the current byte we're checking. 'pad' was
>decremented at the end of each loop iteration. If 'i' was 1 (so we
>checked the final byte), 'pad' could potentially be pointing to the
>first byte of the input buffer, and the decrement would put it one
>byte behind the buffer.
>
>That would be undefined behaviour.
>
>The patch changes it so that 'pad' is the byte we previously checked,
>which allows us to ensure that we only decrement it when we know we
>have a byte to check.
>
>Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/gssapi/krb5/decapsulate.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/lib/gssapi/krb5/decapsulate.c b/lib/gssapi/krb5/decapsulate.c
>index 86085f5695..4e3fcd659e 100644
>--- a/lib/gssapi/krb5/decapsulate.c
>+++ b/lib/gssapi/krb5/decapsulate.c
>@@ -193,13 +193,13 @@ _gssapi_verify_pad(gss_buffer_t wrapped_token,
>     if (wrapped_token->length < 1)
> 	return GSS_S_BAD_MECH;
> 
>-    pad = (u_char *)wrapped_token->value + wrapped_token->length - 1;
>-    padlength = *pad;
>+    pad = (u_char *)wrapped_token->value + wrapped_token->length;
>+    padlength = pad[-1];
> 
>     if (padlength > datalen)
> 	return GSS_S_BAD_MECH;
> 
>-    for (i = padlength; i > 0 && *pad == padlength; i--, pad--)
>+    for (i = padlength; i > 0 && *--pad == padlength; i--)
> 	;
>     if (i != 0)
> 	return GSS_S_BAD_MIC;
>-- 
>2.35.0
>
>
>From cae06f896841a95cb90dd7c438f44ae1254156c3 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Mon, 15 Aug 2022 16:53:55 +1200
>Subject: [PATCH 6/9] CVE-2022-3437 lib/gssapi/krb5: Check the result of
> _gsskrb5_get_mech()
>
>We should make sure that the result of 'total_len - mech_len' won't
>overflow, and that we don't memcmp() past the end of the buffer.
>
>Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/gssapi/krb5/decapsulate.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/lib/gssapi/krb5/decapsulate.c b/lib/gssapi/krb5/decapsulate.c
>index 4e3fcd659e..031a621eab 100644
>--- a/lib/gssapi/krb5/decapsulate.c
>+++ b/lib/gssapi/krb5/decapsulate.c
>@@ -80,6 +80,10 @@ _gssapi_verify_mech_header(u_char **str,
> 
>     if (mech_len != mech->length)
> 	return GSS_S_BAD_MECH;
>+    if (mech_len > total_len)
>+	return GSS_S_BAD_MECH;
>+    if (p - *str > total_len - mech_len)
>+	return GSS_S_BAD_MECH;
>     if (ct_memcmp(p,
> 		  mech->elements,
> 		  mech->length) != 0)
>-- 
>2.35.0
>
>
>From 189b266a4a9f64315d7b75cab709465340dd8918 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Mon, 15 Aug 2022 16:54:23 +1200
>Subject: [PATCH 7/9] CVE-2022-3437 lib/gssapi/krb5: Check buffer length
> against overflow for DES{,3} unwrap
>
>Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/gssapi/krb5/unwrap.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
>diff --git a/lib/gssapi/krb5/unwrap.c b/lib/gssapi/krb5/unwrap.c
>index 61ca29156a..493165bcfa 100644
>--- a/lib/gssapi/krb5/unwrap.c
>+++ b/lib/gssapi/krb5/unwrap.c
>@@ -64,6 +64,8 @@ unwrap_des
> 
>   if (IS_DCE_STYLE(context_handle)) {
>      token_len = 22 + 8 + 15; /* 45 */
>+     if (input_message_buffer->length < token_len)
>+	  return GSS_S_BAD_MECH;
>   } else {
>      token_len = input_message_buffer->length;
>   }
>@@ -76,6 +78,11 @@ unwrap_des
>   if (ret)
>       return ret;
> 
>+  len = (p - (u_char *)input_message_buffer->value)
>+      + 22 + 8;
>+  if (input_message_buffer->length < len)
>+      return GSS_S_BAD_MECH;
>+
>   if (memcmp (p, "\x00\x00", 2) != 0)
>     return GSS_S_BAD_SIG;
>   p += 2;
>@@ -219,6 +226,8 @@ unwrap_des3
> 
>   if (IS_DCE_STYLE(context_handle)) {
>      token_len = 34 + 8 + 15; /* 57 */
>+     if (input_message_buffer->length < token_len)
>+	  return GSS_S_BAD_MECH;
>   } else {
>      token_len = input_message_buffer->length;
>   }
>@@ -231,6 +240,11 @@ unwrap_des3
>   if (ret)
>       return ret;
> 
>+  len = (p - (u_char *)input_message_buffer->value)
>+      + 34 + 8;
>+  if (input_message_buffer->length < len)
>+      return GSS_S_BAD_MECH;
>+
>   if (ct_memcmp (p, "\x04\x00", 2) != 0) /* HMAC SHA1 DES3_KD */
>     return GSS_S_BAD_SIG;
>   p += 2;
>-- 
>2.35.0
>
>
>From 843d68bf31d53e6e98abc168ab85d76ccc4d83b9 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Mon, 10 Oct 2022 20:33:09 +1300
>Subject: [PATCH 8/9] CVE-2022-3437 lib/gssapi/krb5: Check for overflow in
> _gsskrb5_get_mech()
>
>If len_len is equal to total_len - 1 (i.e. the input consists only of a
>0x60 byte and a length), the expression 'total_len - 1 - len_len - 1',
>used as the 'len' parameter to der_get_length(), will overflow to
>SIZE_MAX. Then der_get_length() will proceed to read, unconstrained,
>whatever data follows in memory. Add a check to ensure that doesn't
>happen.
>
>Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/gssapi/krb5/decapsulate.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/lib/gssapi/krb5/decapsulate.c b/lib/gssapi/krb5/decapsulate.c
>index 031a621eab..d7b75a6422 100644
>--- a/lib/gssapi/krb5/decapsulate.c
>+++ b/lib/gssapi/krb5/decapsulate.c
>@@ -54,6 +54,8 @@ _gsskrb5_get_mech (const u_char *ptr,
>     e = der_get_length (p, total_len - 1, &len, &len_len);
>     if (e || 1 + len_len + len != total_len)
> 	return -1;
>+    if (total_len < 1 + len_len + 1)
>+	return -1;
>     p += len_len;
>     if (*p++ != 0x06)
> 	return -1;
>-- 
>2.35.0
>
>
>From 8b181075a089f9cb05f03a6b3679060f565a2e1c Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Wed, 12 Oct 2022 13:57:33 +1300
>Subject: [PATCH 9/9] CVE-2022-3437 lib/gssapi/krb5: Pass correct length to
> _gssapi_verify_pad()
>
>We later subtract 8 when calculating the length of the output message
>buffer. If padlength is excessively high, this calculation can underflow
>and result in a very large positive value.
>
>Now we properly constrain the value of padlength so underflow shouldn't
>be possible.
>
>Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>---
> lib/gssapi/krb5/unwrap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/lib/gssapi/krb5/unwrap.c b/lib/gssapi/krb5/unwrap.c
>index 493165bcfa..64613698fa 100644
>--- a/lib/gssapi/krb5/unwrap.c
>+++ b/lib/gssapi/krb5/unwrap.c
>@@ -124,7 +124,7 @@ unwrap_des
>   } else {
>     /* check pad */
>     ret = _gssapi_verify_pad(input_message_buffer,
>-			     input_message_buffer->length - len,
>+			     input_message_buffer->length - len - 8,
> 			     &padlength);
>     if (ret)
>         return ret;
>@@ -292,7 +292,7 @@ unwrap_des3
>   } else {
>     /* check pad */
>     ret = _gssapi_verify_pad(input_message_buffer,
>-			     input_message_buffer->length - len,
>+			     input_message_buffer->length - len - 8,
> 			     &padlength);
>     if (ret)
>         return ret;
>-- 
>2.35.0
>

Actions: View

Attachments on bug 15134: 17455 | 17474 | 17539 | 17540 | 17541 | 17542 | 17543 | 17549 | 17550 | 17551 | 17552 | 17553 | 17554 | 17555 | 17556 | 17557 | 17558 | 17559 | 17561 | 17564 | 17577 | 17598