From 3e9fa4ca473923ca1d25eea6df831e8327a73d47 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Sun, 15 Mar 2026 12:00:42 +0800 Subject: [PATCH] Fix use-after-free with certain CDP scripts Origins were introduced to group memory/data that can be owned by multiple frames (on the same origin). There's a general idea that the initial "opaque" origin is very transient and should get replaced before any actual JavaScript is executed (because the real origin is setup as soon as we get the header from the response, long before we execute any script). But...with CDP, this guarantee doesn't hold There's nothing stop a CDP script from executing javascript at any point, including while the main page is still being loaded. This can result on allocations made on the opaque origin which is promptly discarded. To solve this, this commit introduced origin takeover. Rather than just transferring any data from one origin (the opaque) to the new one and then deinit' the opaque one (which is what results in user-after-free), the new origin simply maintains a list of opaque origins it has "taken-over"and is responsible for freeing it (in its own deinit). This ensures that any allocation made in the opaque origin remain valid. --- src/browser/js/Context.zig | 7 +++--- src/browser/js/Origin.zig | 49 ++++++++++++++++++++++---------------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 5c58c5cb..97ea9d9c 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -167,12 +167,11 @@ pub fn setOrigin(self: *Context, key: ?[]const u8) !void { const env = self.env; const isolate = env.isolate; + lp.assert(self.origin.rc == 1, "Ref opaque origin", .{ .rc = self.origin.rc }); + const origin = try self.session.getOrCreateOrigin(key); errdefer self.session.releaseOrigin(origin); - - try self.origin.transferTo(origin); - lp.assert(self.origin.rc == 1, "Ref opaque origin", .{ .rc = self.origin.rc }); - self.origin.deinit(env.app); + try origin.takeover(self.origin); self.origin = origin; diff --git a/src/browser/js/Origin.zig b/src/browser/js/Origin.zig index d7e74e4f..180cfd84 100644 --- a/src/browser/js/Origin.zig +++ b/src/browser/js/Origin.zig @@ -68,6 +68,8 @@ temps: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, // if v8 hasn't called the finalizer directly itself. finalizer_callbacks: std.AutoHashMapUnmanaged(usize, *FinalizerCallback) = .empty, +taken_over: std.ArrayList(*Origin), + pub fn init(app: *App, isolate: js.Isolate, key: []const u8) !*Origin { const arena = try app.arena_pool.acquire(); errdefer app.arena_pool.release(arena); @@ -86,14 +88,19 @@ pub fn init(app: *App, isolate: js.Isolate, key: []const u8) !*Origin { .rc = 1, .arena = arena, .key = owned_key, - .globals = .empty, .temps = .empty, + .globals = .empty, + .taken_over = .empty, .security_token = token_global, }; return self; } pub fn deinit(self: *Origin, app: *App) void { + for (self.taken_over.items) |o| { + o.deinit(app); + } + // Call finalizers before releasing anything { var it = self.finalizer_callbacks.valueIterator(); @@ -196,42 +203,44 @@ pub fn createFinalizerCallback( return fc; } -pub fn transferTo(self: *Origin, dest: *Origin) !void { - const arena = dest.arena; +pub fn takeover(self: *Origin, original: *Origin) !void { + const arena = self.arena; - try dest.globals.ensureUnusedCapacity(arena, self.globals.items.len); - for (self.globals.items) |obj| { - dest.globals.appendAssumeCapacity(obj); + try self.globals.ensureUnusedCapacity(arena, self.globals.items.len); + for (original.globals.items) |obj| { + self.globals.appendAssumeCapacity(obj); } - self.globals.clearRetainingCapacity(); + original.globals.clearRetainingCapacity(); { - try dest.temps.ensureUnusedCapacity(arena, self.temps.count()); - var it = self.temps.iterator(); + try self.temps.ensureUnusedCapacity(arena, original.temps.count()); + var it = original.temps.iterator(); while (it.next()) |kv| { - try dest.temps.put(arena, kv.key_ptr.*, kv.value_ptr.*); + try self.temps.put(arena, kv.key_ptr.*, kv.value_ptr.*); } - self.temps.clearRetainingCapacity(); + original.temps.clearRetainingCapacity(); } { - try dest.finalizer_callbacks.ensureUnusedCapacity(arena, self.finalizer_callbacks.count()); - var it = self.finalizer_callbacks.iterator(); + try self.finalizer_callbacks.ensureUnusedCapacity(arena, original.finalizer_callbacks.count()); + var it = original.finalizer_callbacks.iterator(); while (it.next()) |kv| { - kv.value_ptr.*.origin = dest; - try dest.finalizer_callbacks.put(arena, kv.key_ptr.*, kv.value_ptr.*); + kv.value_ptr.*.origin = self; + try self.finalizer_callbacks.put(arena, kv.key_ptr.*, kv.value_ptr.*); } - self.finalizer_callbacks.clearRetainingCapacity(); + original.finalizer_callbacks.clearRetainingCapacity(); } { - try dest.identity_map.ensureUnusedCapacity(arena, self.identity_map.count()); - var it = self.identity_map.iterator(); + try self.identity_map.ensureUnusedCapacity(arena, original.identity_map.count()); + var it = original.identity_map.iterator(); while (it.next()) |kv| { - try dest.identity_map.put(arena, kv.key_ptr.*, kv.value_ptr.*); + try self.identity_map.put(arena, kv.key_ptr.*, kv.value_ptr.*); } - self.identity_map.clearRetainingCapacity(); + original.identity_map.clearRetainingCapacity(); } + + try self.taken_over.append(self.arena, original); } // A type that has a finalizer can have its finalizer called one of two ways.