Heap buffer overflow due to undefined behavior with GCC 8.1 optimizations

Submitted by Vladimir Panteleev

Assigned to Xorg Project Team

Link to original bug (#106985)

Description

This is a follow-up to my email to xorg-devel, which at the time of writing is still stuck in the moderation queue.

Steps to reproduce:

1. Compile 1.20.0 (or git master) with gcc 8.1.1 and meson, with --buildtype release -D b_lto=true
2. Run produced Xorg binary
3. Create layout.xkb with contents:

xkb_keymap { xkb_keycodes { include "evdev+aliases(qwerty)" }; xkb_types { include "complete" }; xkb_compat { include "complete" }; xkb_symbols { include "pc+us+inet(evdev)" }; xkb_geometry { include "teck(teck229)" }; };

4. Run: xkbcomp -I. layout.xkb :0

This causes Xorg to segfault for me.

Why this happens:

1. gcc 8.1 enables new optimizations which exploit undefined behavior. One of these is apparently that strlen() on a pointer taken from an array is assumed to never return a result longer than the array.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86259 https://stackoverflow.com/q/50963054/21501

2. _CheckSetOverlay does exactly this:

        if (XkbAddGeomOverlayKey(ol, row,
                                 (char *) kWire->over,
                                 (char *) kWire->under) == NULL) {

kWire->over and kWire->under are both CARD8[XkbKeyNameLength].

3. XkbAddGeomOverlayKey then calls strlen:

   memcpy(key->under.name, under, min(XkbKeyNameLength, strlen(under))); memcpy(key->over.name, over, min(XkbKeyNameLength, strlen(over)));

GCC assumes that strlen(over) will never be bigger than sizeof(kWire->under), therefore, it optimizes min(XkbKeyNameLength, strlen(over)) to just strlen(over).

4. This results in more than XkbKeyNameLength bytes being written to key->over.name , thus corrupting the heap.

Arch Linux distributes xorg-server binary builds exhibiting this issue.

Version: git