I've been running XQuartz with ASan+UBSan to try to catch some issues some
users have reported, and I stumbled across something below GLUT (specifically,
freeglut 2.8.1), which does:
XConfigureEvent fakeEvent = {0};
...
XPutBackEvent(fgDisplay.Display, (XEvent*)&fakeEvent);
and XPutBackEvent eventually does:
XEvent store = *event;
which overflows the stack on read because:
sizeof(XConfigureEvent) == 88
sizeof(XEvent) == 192
So the problem is clear, but I'm not sure which side needs to change.
What is the contract for Xlib's APIs that take XEvent *? Is Xlib expected to
handle any XEvent "subtype", or does it need to be exactly an XEvent (ie: is it
the client's responsibility to pad it)?
Xlib.h says that the XEvent union "is defined so Xlib can always use the same
sized event structure *internally* (emphasis added), to avoid memory
fragmentation." This makes me think that clients should not use XEvent (and
perhaps XEvent itself should not be API beyond being an opaque pointer).
With a quick grep through our source code, I see some (XEvent *) casting in
many other locations (xterm, cairo, libXt, ...). So is the burden on the
caller to ensure that their events are properly padded into an XEvent, or is
the burden on the recipient of an XEvent to ensure that they determine the
event type before reading the event?
Or is this unique to XPutBackEvent(), and freeglut is abusing XPutBackEvent()
since XPutBackEvent() should be use to "push an event back onto the head of the
display's event queue" (meaning the event should have previously been popped
from the queue and thus be appropriately sized, not something created on the
stack of arbitrary size?
--Jeremy
