From: tokyo4j <hrak1529@gmail.com>
Date: Fri, 3 Jan 2025 15:05:12 +0000 (+0900)
Subject: img: fix UAF on Reconfigure by refcounting
X-Git-Url: https://git.mdlowis.com/?a=commitdiff_plain;h=70fb7138743ba22c0cfd797828fd7477183c39bc;p=proto%2Flabwc.git

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 c83c84ec..4f7b193c 100644
--- 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 22173dbb..fafe9353 100644
--- 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 3600b1c6..6d1d7ae7 100644
--- 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 27d65b9c..0d24a6dd 100644
--- 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 7c92a68e..7874c20d 100644
--- 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 9c24b431..dbad3010 100644
--- 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
 }