From 02200d55eaf01a3a21d52eccfa7eea02f9e8df72 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Wed, 24 Nov 2021 09:21:36 -0500 Subject: [PATCH] Address GCC Bug 95189 memcmp wrongly stripped like strcmp As documented in Russell O'Connor's blog, Heimdal when compiled with some versions of gcc 9 and 10 would generate incorrect behaviors from _gssapi_verify_mic_arcfour(), _gssapi_unwrap_arcfour(), _gssapi_unwrap_iov_arcfour() and _gssapi_unwrap_iov_arcfour(). As a result of the bug, code of the form if (memcmp(a, "\x00\x00\x00\x00")) and cmp = memcmp(a, "\x00\x00\x00\x00") will be compiled as if it were written as if (strcmp(a, "\x00\x00\x00\x00")) and cmp = strcmp(a, "\x00\x00\x00\x00") but not if (memcmp(a, "\x00\x00\x00\x00") != 0) and cmp = (memcmp(a, "\x00\x00\x00\x00") != 0) Bad code is generated whenever one of the parameters to memcmp() is a constant with at least one NUL in the first four octets and the return value is used immediated without a boolean comparison. The gcc bug 95189 has since been fixed. This change applies a defensive programming technique to avoid the broken code generation. Change-Id: I1db2a561735317cb6cac66a0ec9caf5443e65e03 Link: https://r6.ca/blog/20200929T023701Z.html Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95189 Reported-by: Buck Huppmann (buckh@pobox.com) --- lib/gssapi/krb5/arcfour.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gssapi/krb5/arcfour.c b/lib/gssapi/krb5/arcfour.c index 10df05dff..6b209f509 100644 --- a/lib/gssapi/krb5/arcfour.c +++ b/lib/gssapi/krb5/arcfour.c @@ -387,7 +387,7 @@ _gssapi_verify_mic_arcfour(OM_uint32 * minor_status, if (context_handle->more_flags & LOCAL) cmp = memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4); else - cmp = memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4); + cmp = (memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0); memset_s(SND_SEQ, sizeof(SND_SEQ), 0, sizeof(SND_SEQ)); if (cmp != 0) { @@ -658,7 +658,7 @@ OM_uint32 _gssapi_unwrap_arcfour(OM_uint32 *minor_status, if (context_handle->more_flags & LOCAL) cmp = memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4); else - cmp = memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4); + cmp = (memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0); if (cmp != 0) { *minor_status = 0; @@ -1276,7 +1276,7 @@ _gssapi_unwrap_iov_arcfour(OM_uint32 *minor_status, if (ctx->more_flags & LOCAL) { cmp = memcmp(&snd_seq[4], "\xff\xff\xff\xff", 4); } else { - cmp = memcmp(&snd_seq[4], "\x00\x00\x00\x00", 4); + cmp = (memcmp(&snd_seq[4], "\x00\x00\x00\x00", 4) != 0); } if (cmp != 0) { *minor_status = 0;