Are we sure about these changes? By using the variable in the if statement it is no longer a "dummy" variable and I think that the variable name needs to change here.
More so the dl variable is passed for both nitems_return and bytes_after_return, and the final value of dl appears to be the one from bytes_after_return which is typically 0. That means that with this change dwm most likely do not read properties right anymore. On Sat, Jan 10, 2026 at 11:30 AM Hiltjo Posthuma <[email protected]> wrote: > > On Wed, Jan 07, 2026 at 10:02:00PM +0800, Chris Down wrote: > > When getatomprop() is called, it invokes XGetWindowProperty() to > > retrieve an Atom. If the property exists but has zero elements (length > > 0), Xlib returns Success and sets p to a valid, non-NULL memory address > > containing a single null byte. > > > > However, dl (that is, the number of items) is 0. dwm blindly casts p to > > Atom* and dereferences it. While Xlib guarantees that p is safe to read > > as a string (that is, it is null-terminated), it does _not_ guarantee it > > is safe to read as an Atom (an unsigned long). > > > > The Atom type is a typedef for unsigned long. Reading an Atom (which > > thus will either likely be 4 or 8 bytes) from a 1-byte allocated buffer > > results in a heap buffer overflow. Since property content is user > > controlled, this allows any client to trigger an out of bounds read > > simply by setting a property with format 32 and length 0. > > > > An example client which reliably crashes dwm under ASAN: > > > > #include <X11/Xlib.h> > > #include <X11/Xatom.h> > > #include <stdio.h> > > #include <stdlib.h> > > #include <unistd.h> > > > > int main(void) { > > Display *d; > > Window root, w; > > Atom net_wm_state; > > > > d = XOpenDisplay(NULL); > > if (!d) return 1; > > > > root = DefaultRootWindow(d); > > w = XCreateSimpleWindow(d, root, 10, 10, 200, 200, 1, 0, 0); > > net_wm_state = XInternAtom(d, "_NET_WM_STATE", False); > > if (net_wm_state == None) return 1; > > > > XChangeProperty(d, w, net_wm_state, XA_ATOM, 32, > > PropModeReplace, NULL, 0); > > XMapWindow(d, w); > > XSync(d, False); > > sleep(1); > > > > XCloseDisplay(d); > > return 0; > > } > > > > In order to avoid this, check that the number of items returned is > > greater than zero before dereferencing the pointer. > > --- > > dwm.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/dwm.c b/dwm.c > > index 4f345ee..8f4fa75 100644 > > --- a/dwm.c > > +++ b/dwm.c > > @@ -870,7 +870,8 @@ getatomprop(Client *c, Atom prop) > > > > if (XGetWindowProperty(dpy, c->win, prop, 0L, sizeof atom, False, > > XA_ATOM, > > &da, &di, &dl, &dl, &p) == Success && p) { > > - atom = *(Atom *)p; > > + if (dl > 0) > > + atom = *(Atom *)p; > > XFree(p); > > } > > return atom; > > -- > > 2.52.0 > > > > > > Hi Chris, > > Thanks for the patch and very clear reproducable description. > > I pushed the patch to the repo. > > -- > Kind regards, > Hiltjo >
