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.
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);
+ 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("");
- 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);
TAILQ_INSERT_TAIL(&cc->nameq, wn, entry);
/* Garbage collection. */
