On Sun, Mar 15, 2020 at 10:09:53AM -0400, Okan Demirmen wrote:
> On Sat 2020.03.14 at 18:33 +0100, Tim van der Molen wrote:
> > Stefan Sperling (2020-03-14 18:12 +0100):
> > > On Sat, Mar 14, 2020 at 05:36:46PM +0100, Otto Moerbeek wrote:
> > > > Looking a the diff, I do see a use-after-free.
> > >
> > > The problem is the use of TAILQ_FOREACH(wn, ...) instead of
> > > TAILQ_FOREACH_SAFE(wn, ...) in combination with free(wn), correct?
> >
> > Most likely, yes.
>
> Revised diff using TAILQ_FOREACH_SAFE.
Doesn't seem right.
>
> Index: client.c
> ===================================================================
> RCS file: /home/open/cvs/xenocara/app/cwm/client.c,v
> retrieving revision 1.260
> diff -u -p -r1.260 client.c
> --- client.c 14 Mar 2020 16:11:09 -0000 1.260
> +++ client.c 14 Mar 2020 18:36:49 -0000
> @@ -667,22 +667,24 @@ client_close(struct client_ctx *cc)
> void
> client_set_name(struct client_ctx *cc)
> {
> - struct winname *wn;
> - char *newname;
> + struct winname *wn, *wmnxt;
> int i = 0;
>
> - if (!xu_get_strprop(cc->win, ewmh[_NET_WM_NAME], &newname))
> - if (!xu_get_strprop(cc->win, XA_WM_NAME, &newname))
> - newname = xstrdup("");
> + free(cc->name);
cc->name is freed
> + if (!xu_get_strprop(cc->win, ewmh[_NET_WM_NAME], &cc->name))
> + if (!xu_get_strprop(cc->win, XA_WM_NAME, &cc->name))
> + cc->name = xstrdup("");
And only re-assigned on certain conditions
>
> - TAILQ_FOREACH(wn, &cc->nameq, entry) {
> - if (strcmp(wn->name, newname) == 0)
> + TAILQ_FOREACH_SAFE(wn, &cc->nameq, entry, wmnxt) {
> + if (strcmp(wn->name, cc->name) == 0) {
> TAILQ_REMOVE(&cc->nameq, wn, entry);
> + free(wn->name);
> + free(wn);
> + }
> i++;
> }
> - cc->name = newname;
> wn = xmalloc(sizeof(*wn));
> - wn->name = xstrdup(newname);
> + wn->name = xstrdup(cc->name);
and then used unconditioanlly
> TAILQ_INSERT_TAIL(&cc->nameq, wn, entry);
>
> /* Garbage collection. */
