Skip to content

Commit 3ada6ee

Browse files
fix: updateStatus captured a full *StatusBar in a pending idle callback (theoretical UAF)
Flagged by the GTK crash-hunt investigation, not reproduced but real: of StatusBar's five g_idle_add-based update methods, updateStatus was the only one capturing self_ptr: *StatusBar and dereferencing the whole struct from inside the callback — if StatusBar.deinit() ran while the callback was still queued (idle callbacks aren't guaranteed to run before the next main-loop iteration), that's a real use-after-free. The other four methods (updateProgress/pulseProgress/updateVoice/updateEngine) already capture only the specific widget pointers + allocator they need, which survive freeing the StatusBar struct itself just fine — made updateStatus match that existing, safer pattern instead of introducing new tracking machinery. The narrower "GTK widget itself destroyed before the idle callback fires" risk (present in all five methods equally, not just updateStatus) is a different, harder problem needing weak-pointer tracking on the widgets — left as documented backlog, not fixed here, matching the low-priority/ unreproduced assessment this was originally flagged with. Verified: full test suite passes (46/47, 1 pre-existing unrelated skip), smoke-tested a real run with no new warnings/crashes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 92ae391 commit 3ada6ee

1 file changed

Lines changed: 28 additions & 16 deletions

File tree

src/kit/components/status_bar.zig

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -102,39 +102,49 @@ pub const StatusBar = struct {
102102
}
103103

104104
pub fn updateStatus(self: *StatusBar, message: []const u8, is_error: bool) void {
105+
// Captures only the specific widgets + allocator this callback needs
106+
// (same pattern as updateVoice/updateEngine below), not a `*StatusBar`
107+
// — this used to hold `self_ptr: *StatusBar` and dereference the
108+
// whole struct from inside the idle callback, which would be a real
109+
// use-after-free if `StatusBar.deinit()` ran while this callback was
110+
// still queued (idle callbacks aren't guaranteed to run before the
111+
// next main-loop iteration, so that's not just a theoretical
112+
// ordering). Every field needed is a plain widget pointer or a copy
113+
// of the allocator interface, neither of which is invalidated by
114+
// freeing the StatusBar struct itself.
105115
const UpdateUI = struct {
106-
self_ptr: *StatusBar,
116+
status_label: ?*ffi.GtkLabel,
117+
status_icon: ?*ffi.GtkWidget,
118+
allocator: std.mem.Allocator,
107119
msg: [*:0]const u8,
108120
err: bool,
109121
fn update(ptr: ffi.gpointer) callconv(.c) bool {
110122
const ctx: *@This() = @ptrCast(@alignCast(ptr));
111-
const status_bar = ctx.self_ptr;
112123
const msg_ptr = ctx.msg;
113124
const is_err = ctx.err;
114125

115126
const color = if (is_err) "#f7768e" else "#7aa2f7";
116127
const icon = if (is_err) "dialog-error-symbolic" else "emblem-system-symbolic";
117128

118-
const escaped = text.escape(status_bar.allocator, std.mem.span(msg_ptr)) catch {
119-
status_bar.allocator.free(std.mem.span(msg_ptr));
120-
status_bar.allocator.destroy(ctx);
129+
const escaped = text.escape(ctx.allocator, std.mem.span(msg_ptr)) catch {
130+
ctx.allocator.free(std.mem.span(msg_ptr));
131+
ctx.allocator.destroy(ctx);
121132
return false;
122133
};
123-
defer status_bar.allocator.free(escaped);
134+
defer ctx.allocator.free(escaped);
124135

125-
const fmt_markup = std.fmt.allocPrintSentinel(status_bar.allocator, "<span foreground='{s}'>{s}</span>", .{ color, escaped }, 0) catch {
126-
status_bar.allocator.free(escaped);
127-
status_bar.allocator.free(std.mem.span(msg_ptr));
128-
status_bar.allocator.destroy(ctx);
136+
const fmt_markup = std.fmt.allocPrintSentinel(ctx.allocator, "<span foreground='{s}'>{s}</span>", .{ color, escaped }, 0) catch {
137+
ctx.allocator.free(std.mem.span(msg_ptr));
138+
ctx.allocator.destroy(ctx);
129139
return false;
130140
};
131141

132-
ffi.gtk_label_set_markup(status_bar.status_label, fmt_markup.ptr);
133-
ffi.gtk_image_set_from_icon_name(status_bar.status_icon, icon);
142+
ffi.gtk_label_set_markup(ctx.status_label, fmt_markup.ptr);
143+
ffi.gtk_image_set_from_icon_name(ctx.status_icon, icon);
134144

135-
status_bar.allocator.free(fmt_markup);
136-
status_bar.allocator.free(std.mem.span(msg_ptr));
137-
status_bar.allocator.destroy(ctx);
145+
ctx.allocator.free(fmt_markup);
146+
ctx.allocator.free(std.mem.span(msg_ptr));
147+
ctx.allocator.destroy(ctx);
138148
return false;
139149
}
140150
};
@@ -145,7 +155,9 @@ pub const StatusBar = struct {
145155
return;
146156
};
147157
ctx.* = .{
148-
.self_ptr = self,
158+
.status_label = self.status_label,
159+
.status_icon = self.status_icon,
160+
.allocator = self.allocator,
149161
.msg = msg,
150162
.err = is_error,
151163
};

0 commit comments

Comments
 (0)