From 6d77b023fcfc246164c8061fd9ed31c3d3d53096 Mon Sep 17 00:00:00 2001 From: Joseph Sutton 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 --- 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 5dfce68626a8587ddce66ab1caa78d242b6c626a Mon Sep 17 00:00:00 2001 From: Joseph Sutton 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 --- 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 12c124827606f2e9fb5cadc45fc1661a85282bfb Mon Sep 17 00:00:00 2001 From: Joseph Sutton 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 --- 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 2c8d4290f665c4eba58f711500feea821b25ec6c Mon Sep 17 00:00:00 2001 From: Joseph Sutton 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 --- 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 d98688a0c291094299fea4f532e68acd900f0cb0 Mon Sep 17 00:00:00 2001 From: Joseph Sutton 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. Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton --- 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 dceb575f3ca369a561d8c597453d2f0250ee73c6 Mon Sep 17 00:00:00 2001 From: Joseph Sutton 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 --- 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 531df9f4d6ba70b47eb21ef7ae2dfaf4e56c6e24 Mon Sep 17 00:00:00 2001 From: Joseph Sutton 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 --- 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..1459b623da 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 (len > input_message_buffer->length) + 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 (len > input_message_buffer->length) + 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 a11486ca3a9a66ecb83ce437717a28a00f4f6f9f Mon Sep 17 00:00:00 2001 From: Joseph Sutton 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 --- 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 3525b7cacf00562fe6204e04e6af58e7eafe4768 Mon Sep 17 00:00:00 2001 From: Joseph Sutton 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 --- 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 1459b623da..efbe392c91 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