From 924fbcb74ae5434afa7ce4603cd85ebcbdcccad5 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 28 Nov 2023 15:19:04 +1000 Subject: [PATCH xserver] Xi: allocate enough XkbActions for our buttons button->xkb_acts is supposed to be an array sufficiently large for all our buttons, not just a single XkbActions struct. Allocating insufficient memory here means when we memcpy() later in XkbSetDeviceInfo we write into memory that wasn't ours to begin with, leading to the usual security ooopsiedaisies. CVE-2023-6377, ZDI-CAN-22412, ZDI-CAN-22413 This vulnerability was discovered by: Jan-Niklas Sohn working with Trend Micro Zero Day Initiative --- Xi/exevents.c | 12 ++++++------ dix/devices.c | 10 ++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) --- a/Xi/exevents.c +++ a/Xi/exevents.c @@ -611,13 +611,13 @@ DeepCopyPointerClasses(DeviceIntPtr from, DeviceIntPtr to) } if (from->button->xkb_acts) { - if (!to->button->xkb_acts) { - to->button->xkb_acts = calloc(1, sizeof(XkbAction)); - if (!to->button->xkb_acts) - FatalError("[Xi] not enough memory for xkb_acts.\n"); - } + size_t maxbuttons = max(to->button->numButtons, from->button->numButtons); + to->button->xkb_acts = xnfreallocarray(to->button->xkb_acts, + maxbuttons, + sizeof(XkbAction)); + memset(to->button->xkb_acts, 0, maxbuttons * sizeof(XkbAction)); memcpy(to->button->xkb_acts, from->button->xkb_acts, - sizeof(XkbAction)); + from->button->numButtons * sizeof(XkbAction)); } else { free(to->button->xkb_acts); --- a/dix/devices.c +++ a/dix/devices.c @@ -2530,6 +2530,8 @@ RecalculateMasterButtons(DeviceIntPtr slave) if (master->button && master->button->numButtons != maxbuttons) { int i; + int last_num_buttons = master->button->numButtons; + DeviceChangedEvent event = { .header = ET_Internal, .type = ET_DeviceChanged, @@ -2540,6 +2542,14 @@ RecalculateMasterButtons(DeviceIntPtr slave) }; master->button->numButtons = maxbuttons; + if (last_num_buttons < maxbuttons) { + master->button->xkb_acts = xnfreallocarray(master->button->xkb_acts, + maxbuttons, + sizeof(XkbAction)); + memset(&master->button->xkb_acts[last_num_buttons], + 0, + (maxbuttons - last_num_buttons) * sizeof(XkbAction)); + } memcpy(&event.buttons.names, master->button->labels, maxbuttons * sizeof(Atom)); --