From 504cf509ea07a130bfd2540804e70cdf0e84b219 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 Reviewed-by: Andrew Bartlett --- 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 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 Reviewed-by: Andrew Bartlett --- 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 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 Reviewed-by: Andrew Bartlett --- 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 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 Reviewed-by: Andrew Bartlett --- 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 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 Reviewed-by: Andrew Bartlett --- 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 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 Reviewed-by: Andrew Bartlett --- 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 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 Reviewed-by: Andrew Bartlett --- 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 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 Reviewed-by: Andrew Bartlett --- 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 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 Reviewed-by: Andrew Bartlett --- 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