Re: [hackers] [PATCH] dwm: Fix heap buffer overflow in getatomprop

From: Stein Gunnar Bakkeby <bakkeby_AT_gmail.com>
Date: Sun, 11 Jan 2026 21:35:14 +0100

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 <hiltjo_AT_codemadness.org> 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
> > _AT_@ -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
>
Received on Sun Jan 11 2026 - 21:35:14 CET