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.
This commit is contained in:
tokyo4j 2025-01-04 00:05:12 +09:00 committed by Consolatis
parent 90a8c3e793
commit 70fb713874
6 changed files with 32 additions and 11 deletions

View file

@ -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);

View file

@ -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 */

View file

@ -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 */

View file

@ -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;

View file

@ -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);
}

View file

@ -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
}