From 74627d13c75cab7aa73c0e713feae0934e780ba0 Mon Sep 17 00:00:00 2001 From: Michal Srb Date: Thu, 21 Jun 2018 13:44:04 +0200 Subject: [PATCH] xkb: Fix heap overflow caused by optimized away min. Calling strlen on char[4] that does not need to contain '\0' is wrong and X server may end up running into uninitialized memory. In addition GCC 8 is clever enough that it knows that strlen on char[4] can return 0, 1, 2, 3 or cause undefined behavior. With this knowledge it can optimize away the min(..., 4). In reality it can cause the memcpy to be called with bigger size than 4 and overflow the destination buffer. Fixes: 83913de25d35 (xkb: Silence some compiler warnings) Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/288 Signed-off-by: Matt Turner --- xkb/XKBGAlloc.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/xkb/XKBGAlloc.c b/xkb/XKBGAlloc.c index 8958b0c52..f0cda24fe 100644 --- a/xkb/XKBGAlloc.c +++ b/xkb/XKBGAlloc.c @@ -588,8 +588,7 @@ XkbAddGeomKeyAlias(XkbGeometryPtr geom, char *aliasStr, char *realStr) i++, alias++) { if (strncmp(alias->alias, aliasStr, XkbKeyNameLength) == 0) { memset(alias->real, 0, XkbKeyNameLength); - memcpy(alias->real, realStr, - min(XkbKeyNameLength, strlen(realStr))); + memcpy(alias->real, realStr, strnlen(realStr, XkbKeyNameLength)); return alias; } } @@ -599,8 +598,8 @@ XkbAddGeomKeyAlias(XkbGeometryPtr geom, char *aliasStr, char *realStr) } alias = &geom->key_aliases[geom->num_key_aliases]; memset(alias, 0, sizeof(XkbKeyAliasRec)); - memcpy(alias->alias, aliasStr, min(XkbKeyNameLength, strlen(aliasStr))); - memcpy(alias->real, realStr, min(XkbKeyNameLength, strlen(realStr))); + memcpy(alias->alias, aliasStr, strnlen(aliasStr, XkbKeyNameLength)); + memcpy(alias->real, realStr, strnlen(realStr, XkbKeyNameLength)); geom->num_key_aliases++; return alias; } @@ -815,8 +814,8 @@ XkbAddGeomOverlayKey(XkbOverlayPtr overlay, (_XkbAllocOverlayKeys(row, 1) != Success)) return NULL; key = &row->keys[row->num_keys]; - memcpy(key->under.name, under, min(XkbKeyNameLength, strlen(under))); - memcpy(key->over.name, over, min(XkbKeyNameLength, strlen(over))); + memcpy(key->under.name, under, strnlen(under, XkbKeyNameLength)); + memcpy(key->over.name, over, strnlen(over, XkbKeyNameLength)); row->num_keys++; return key; }