From f6afdf6b1395d1c7b87f4bd83653a5baa5fd5e67 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 15 Aug 2022 16:53:45 +1200 Subject: [PATCH 1/3] 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. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- source4/heimdal/lib/gssapi/krb5/decapsulate.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source4/heimdal/lib/gssapi/krb5/decapsulate.c b/source4/heimdal/lib/gssapi/krb5/decapsulate.c index 86085f56950..4e3fcd659e9 100644 --- a/source4/heimdal/lib/gssapi/krb5/decapsulate.c +++ b/source4/heimdal/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.25.1 From 4d41c443b67b938730657d25c3a812a270ec540e Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 15 Aug 2022 16:53:55 +1200 Subject: [PATCH 2/3] 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. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- source4/heimdal/lib/gssapi/krb5/decapsulate.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source4/heimdal/lib/gssapi/krb5/decapsulate.c b/source4/heimdal/lib/gssapi/krb5/decapsulate.c index 4e3fcd659e9..031a621eabc 100644 --- a/source4/heimdal/lib/gssapi/krb5/decapsulate.c +++ b/source4/heimdal/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.25.1 From 78214c654bd6689d1813bbea5cfa13e9c78be324 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 15 Aug 2022 16:54:23 +1200 Subject: [PATCH 3/3] lib/gssapi/krb5: Check buffer length against overflow for DES{,3} unwrap BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- source4/heimdal/lib/gssapi/krb5/unwrap.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/source4/heimdal/lib/gssapi/krb5/unwrap.c b/source4/heimdal/lib/gssapi/krb5/unwrap.c index b3da35ee9e2..b1c09521c4f 100644 --- a/source4/heimdal/lib/gssapi/krb5/unwrap.c +++ b/source4/heimdal/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; @@ -215,6 +222,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; } @@ -227,6 +236,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 (memcmp (p, "\x04\x00", 2) != 0) /* HMAC SHA1 DES3_KD */ return GSS_S_BAD_SIG; p += 2; -- 2.25.1