Re: [hackers] [dwm][PATCH] Avoid unsigned integer underflow in drw_text()

From: Raymond Cole <rc_AT_wolog.xyz>
Date: Tue, 29 Oct 2024 01:16:24 +0000

On 24/10/28 08:57PM, Hiltjo Posthuma wrote:
> On Mon, Oct 28, 2024 at 12:34:55AM +0000, Raymond Cole wrote:
> > ---
> > drw.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drw.c b/drw.c
> > index 344de61..c41e6af 100644
> > --- a/drw.c
> > +++ b/drw.c
> > _AT_@ -248,6 +248,8 @@ drw_text(Drw *drw, int x, int y, unsigned int w, unsigned int h, unsigned int lp
> > } else {
> > XSetForeground(drw->dpy, drw->gc, drw->scheme[invert ? ColFg : ColBg].pixel);
> > XFillRectangle(drw->dpy, drw->drawable, drw->gc, x, y, w, h);
> > + if (w < lpad)
> > + return x + w;
> > d = XftDrawCreate(drw->dpy, drw->drawable,
> > DefaultVisual(drw->dpy, drw->screen),
> > DefaultColormap(drw->dpy, drw->screen));
> > --
> > 2.46.2
> >
> >
>
> Thanks, it looks good, but I'll review it later.
>
> In the meantime do you perhaps also have a bit more information about it? Does
> this fix a bug or do you have a reproducable case?
>
> This helps in reviewing it better/faster, etc.
>
> --
> Kind regards,
> Hiltjo
>
In dwm, the width argument to be passed to drw_text when drawing window
title is computed by subtracting tags wdith, layout symbol width and
status text width from monitor width, and dwm doesn't draw the window
title if the result is not bigger than bh, which, on purpose or by chance,
prevents calling drw_text with a width smaller than lpad. I removed
that apparently arbitrary comparison with bh, only to discover that
when the width available to drawing window title becomes very small,
all of a sudden drw_text behaves as if the width is unlimited, and the
cause is that unsigned integer underflow.

I don't think at this point this problem affects dwm, but drw is
general-purpose so this improvement in robustness, with the addition of
just two short lines, is well worth it.
Received on Tue Oct 29 2024 - 02:16:24 CET