img: fix UAF on Reconfigure by refcounting

Before this commit, there was a use-after-free bug on Reconfigure:
- theme_finish() destroys lab_imgs for titlebar icons
- For some reason, undecorate() calls _create_buffer() in
  scaled-img-buffer.c, which calls img_render() on a destroyed lab_img.

So in this commit, the lifetime of lab_img is expanded to when the
scaled_img_buffers referencing it are all destroyed. This is achieved by
calling lab_img_copy() when setting a lab_img to scaled_img_buffer and
calling lab_img_destroy() when clearing a lab_img.

Now that scaled_img_buffer.img are always different, lab_img_equal() is
added to compare the content of scaled_img_buffer.img.

diff --git a/include/common/scaled-img-buffer.h b/include/common/scaled-img-buffer.h
index c83c84ec9ac11f909fa20bd2e7090d8b25c1b9e2..4f7b193cb616ff10535db0526f62e06ff25fe98e 100644 (file)
--- a/include/common/scaled-img-buffer.h
+++ b/include/common/scaled-img-buffer.h
@@ -23,6 +23,9 @@ struct scaled_img_buffer {
  * display. It gets destroyed automatically when the backing scaled_scene_buffer
  * is being destroyed which in turn happens automatically when the backing
  * wlr_scene_buffer (or one of its parents) is being destroyed.
+ *
+ * This function clones the lab_img passed as the image source, so callers are
+ * free to destroy it.
  */
 struct scaled_img_buffer *scaled_img_buffer_create(struct wlr_scene_tree *parent,
        struct lab_img *img, int width, int height, int padding);

diff --git a/include/img/img.h b/include/img/img.h
index 22173dbb269783798dbb80feb9f7d96f4c785292..fafe9353f8295a81ed172fadcf262688500e129f 100644 (file)
--- a/include/img/img.h
+++ b/include/img/img.h
@@ -3,6 +3,7 @@
 #define LABWC_IMG_H
 
 #include <cairo.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <wayland-util.h>
 
@@ -72,4 +73,9 @@ struct lab_data_buffer *lab_img_render(struct lab_img *img,
  */
 void lab_img_destroy(struct lab_img *img);
 
+/**
+ * lab_img_equal() - Returns true if two images draw the same content
+ */
+bool lab_img_equal(struct lab_img *img_a, struct lab_img *img_b);
+
 #endif /* LABWC_IMG_H */

diff --git a/include/ssd-internal.h b/include/ssd-internal.h
index 3600b1c649cb22af328bb96d7001df8225b2da4a..6d1d7ae746d1d05536e9a16ce584d166683a0d95 100644 (file)
--- a/include/ssd-internal.h
+++ b/include/ssd-internal.h
@@ -79,7 +79,6 @@ struct ssd {
                } title;
 
                char *app_id;
-               struct lab_img *icon_img;
        } state;
 
        /* An invisible area around the view which allows resizing */

diff --git a/src/common/scaled-img-buffer.c b/src/common/scaled-img-buffer.c
index 27d65b9c25b24c32c4ab8d3ff44105b604a0f54b..0d24a6dd082071fa0249b3c3a0564ea5ee006a56 100644 (file)
--- a/src/common/scaled-img-buffer.c
+++ b/src/common/scaled-img-buffer.c
@@ -26,6 +26,7 @@ static void
 _destroy(struct scaled_scene_buffer *scaled_buffer)
 {
        struct scaled_img_buffer *self = scaled_buffer->data;
+       lab_img_destroy(self->img);
        free(self);
 }
 
@@ -36,7 +37,7 @@ _equal(struct scaled_scene_buffer *scaled_buffer_a,
        struct scaled_img_buffer *a = scaled_buffer_a->data;
        struct scaled_img_buffer *b = scaled_buffer_b->data;
 
-       return a->img == b->img
+       return lab_img_equal(a->img, b->img)
                && a->width == b->width
                && a->height == b->height
                && a->padding == b->padding;
@@ -52,12 +53,13 @@ struct scaled_img_buffer *
 scaled_img_buffer_create(struct wlr_scene_tree *parent, struct lab_img *img,
        int width, int height, int padding)
 {
+       assert(img);
        struct scaled_scene_buffer *scaled_buffer = scaled_scene_buffer_create(
                parent, &impl, &cached_buffers, /* drop_buffer */ true);
        struct scaled_img_buffer *self = znew(*self);
        self->scaled_buffer = scaled_buffer;
        self->scene_buffer = scaled_buffer->scene_buffer;
-       self->img = img;
+       self->img = lab_img_copy(img);
        self->width = width;
        self->height = height;
        self->padding = padding;
@@ -73,7 +75,9 @@ void
 scaled_img_buffer_update(struct scaled_img_buffer *self, struct lab_img *img,
        int width, int height, int padding)
 {
-       self->img = img;
+       assert(img);
+       lab_img_destroy(self->img);
+       self->img = lab_img_copy(img);
        self->width = width;
        self->height = height;
        self->padding = padding;

diff --git a/src/img/img.c b/src/img/img.c
index 7c92a68e7a3b605c61bcff12bf6d1076d945c76a..7874c20d1edc632cb71f1929a72d1cdee9d19d50 100644 (file)
--- a/src/img/img.c
+++ b/src/img/img.c
@@ -218,3 +218,18 @@ lab_img_destroy(struct lab_img *img)
        wl_array_release(&img->modifiers);
        free(img);
 }
+
+bool
+lab_img_equal(struct lab_img *img_a, struct lab_img *img_b)
+{
+       if (img_a == img_b) {
+               return true;
+       }
+       if (!img_a || !img_b || img_a->cache != img_b->cache
+                       || img_a->modifiers.size != img_b->modifiers.size) {
+               return false;
+       }
+       return img_a->modifiers.size == 0
+               || !memcmp(img_a->modifiers.data, img_b->modifiers.data,
+                       img_a->modifiers.size);
+}

diff --git a/src/ssd/ssd-titlebar.c b/src/ssd/ssd-titlebar.c
index 9c24b431a74c5de47e671f1798747fc8ef162d5f..dbad3010e619961557ac4df6750202dab4b66557 100644 (file)
--- a/src/ssd/ssd-titlebar.c
+++ b/src/ssd/ssd-titlebar.c
@@ -345,9 +345,6 @@ ssd_titlebar_destroy(struct ssd *ssd)
        if (ssd->state.app_id) {
                zfree(ssd->state.app_id);
        }
-       if (ssd->state.icon_img) {
-               lab_img_destroy(ssd->state.icon_img);
-       }
 
        wlr_scene_node_destroy(&ssd->titlebar.tree->node);
        ssd->titlebar.tree = NULL;
@@ -642,10 +639,7 @@ ssd_update_window_icon(struct ssd *ssd)
                }
        } FOR_EACH_END
 
-       if (ssd->state.icon_img) {
-               lab_img_destroy(ssd->state.icon_img);
-       }
-       ssd->state.icon_img = icon_img;
+       lab_img_destroy(icon_img);
 #endif
 }