uaf: Window reuse on iframe/popup navigation

This commit re-uses the allocated *Window on frame/popup navigation. It's simple
to understand why this is needed:

```
var opener = window.open('page1.html');
opener.location = 'page2.html'
opener <-- should still be valid
```

What's harder to understand is why this mostly works and why it started to fail.
This mostly works because the *Window is allocated on the page's arena and thus
lives for the duration of the page AND, because we re-use the frame address,
`window._frame` always points to a valid/current Frame (as far as JS is
concerned).

Why is started to fail is because of the move to rust-url (1). On frame.deinit
we now free the underlying window._location._url._url rust-owned memory. So
while the window stays alive and most things work, getting that URL is a UAF.
By re-using the window, we don't change the lifetime of the rust-owned URL, but
we do give the existing window a new location + url. (Which is correct, as
opener.location should reflect the new URL post-navigation).

(1) https://github.com/lightpanda-io/browser/pull/2658
This commit is contained in:
Karl Seguin
2026-06-15 12:12:31 +08:00
parent ba433059c0
commit f94d69a8cd
7 changed files with 108 additions and 13 deletions

View File

@@ -288,11 +288,24 @@ pub const HttpHeader = struct {
value: []const u8,
};
pub fn init(self: *Frame, frame_id: u32, page: *Page, parent: ?*Frame) !void {
pub const InitOpts = struct {
parent: ?*Frame = null,
// When a frame/popup re-navigates, we should preserve the same window.
// There are a couple reasons for this. First, iframe.contentWindow should
// maintain the same identity (which we don't correctly do now, but this
// is a first step). Secondly, a reference to the window can be acquired
// prior to navigation, and then used after. So it should remain valid.
reuse_window: ?*Window = null,
};
pub fn init(self: *Frame, frame_id: u32, page: *Page, opts: InitOpts) !void {
if (comptime IS_DEBUG) {
log.debug(.frame, "frame.init", .{});
}
const parent = opts.parent;
const session = page.session;
const call_arena = try session.getArena(.medium, "call_arena");
errdefer session.releaseArena(call_arena);
@@ -341,7 +354,7 @@ pub fn init(self: *Frame, frame_id: u32, page: *Page, parent: ?*Frame) !void {
});
}
self.window = try factory.eventTarget(Window{
const window_template = Window{
._frame = self,
._proto = undefined,
._document = self.document,
@@ -350,7 +363,16 @@ pub fn init(self: *Frame, frame_id: u32, page: *Page, parent: ?*Frame) !void {
._screen = screen,
._visual_viewport = visual_viewport,
._cross_origin_wrapper = undefined,
});
};
if (opts.reuse_window) |w| {
const proto = w._proto;
w.* = window_template;
w._proto = proto;
self.window = w;
} else {
self.window = try factory.eventTarget(window_template);
}
self.window._cross_origin_wrapper = .{ .window = self.window };
self._style_manager = try StyleManager.init(self);
@@ -1463,7 +1485,7 @@ pub fn iframeAddedCallback(self: *Frame, iframe: *IFrame) !void {
const new_frame = try self.arena.create(Frame);
const frame_id = session.nextFrameId();
try Frame.init(new_frame, frame_id, self._page, self);
try Frame.init(new_frame, frame_id, self._page, .{ .parent = self });
errdefer new_frame.deinit();
self._pending_loads += 1;
@@ -1587,7 +1609,7 @@ pub fn openPopup(self: *Frame, opts: OpenPopupOpts) !*Frame {
errdefer page.frame_arena.destroy(popup);
const frame_id = session.nextFrameId();
try Frame.init(popup, frame_id, page, null);
try Frame.init(popup, frame_id, page, .{});
errdefer popup.deinit();
popup.window._opener = opts.opener;

View File

@@ -126,7 +126,7 @@ pub fn init(self: *Page, session: *Session, frame_id: u32) !void {
};
self.queued_navigation = &self.queued_navigation_1;
try Frame.init(&self.frame, frame_id, self, null);
try Frame.init(&self.frame, frame_id, self, .{});
}
// Tear down the Page and its root Frame. Equivalent to the old

View File

@@ -530,6 +530,7 @@ fn _processFrameNavigation(self: *Session, frame: *Frame, qn: *QueuedNavigation)
}
const frame_id = frame._frame_id;
const reuse_window = frame.window;
const page = self.currentPage().?;
frame.deinit();
frame.* = undefined;
@@ -546,7 +547,7 @@ fn _processFrameNavigation(self: *Session, frame: *Frame, qn: *QueuedNavigation)
}
}
try Frame.init(frame, frame_id, page, parent);
try Frame.init(frame, frame_id, page, .{ .parent = parent, .reuse_window = reuse_window });
errdefer {
if (parent_notified) {
parent._pending_loads -= 1;
@@ -563,14 +564,16 @@ fn _processFrameNavigation(self: *Session, frame: *Frame, qn: *QueuedNavigation)
};
}
// Re-navigates a popup Frame in place. The Frame pointer stays stable
// (scripts in the opener may hold a cached Window ref — though the Window
// object inside is replaced, matching how iframes behave on navigation).
// Re-navigates a popup Frame in place. Both the Frame pointer and its Window
// stay stable across the re-init, so a cached `window.open()` return value keeps
// a valid `.location` instead of dangling against the freed one.
fn processPopupNavigation(self: *Session, frame: *Frame, qn: *QueuedNavigation) !void {
// Preserve popup identity fields. _name lives in the Page arena and
// survives Frame.deinit; _opener is just a pointer.
const saved_name = frame.window._name;
const saved_opener = frame.window._opener;
const reuse_window = frame.window;
const saved_name = reuse_window._name;
const saved_opener = reuse_window._opener;
const frame_id = frame._frame_id;
const page = self.currentPage().?;
@@ -587,7 +590,7 @@ fn processPopupNavigation(self: *Session, frame: *Frame, qn: *QueuedNavigation)
}
}
try Frame.init(frame, frame_id, page, null);
try Frame.init(frame, frame_id, page, .{ .reuse_window = reuse_window });
errdefer frame.deinit();
frame.window._name = saved_name;

View File

@@ -221,6 +221,20 @@ pub fn deinit(self: *Context) void {
// have a dangling pointer to our freed Context struct.
v8.v8__Context__SetAlignedPointerInEmbedderData(entered.handle, 1, null);
// Evict the window's entry from the identity map. The window pointer is the
// identity-map key (see Env.createContext), and an in-place navigation
// reuses the same Window address — a lingering entry would make the next
// context's getOrPut see `found_existing` and skip registering its global.
switch (self.global) {
.frame => |frame| {
if (self.identity.identity_map.fetchRemove(@intFromPtr(frame.window))) |kv| {
var global = kv.value;
v8.v8__Global__Reset(&global);
}
},
.worker => {},
}
v8.v8__Global__Reset(&self.handle);
env.isolate.notifyContextDisposed();
// There can be other tasks associated with this context that we need to

View File

@@ -0,0 +1,2 @@
<!DOCTYPE html>
<title>win_a</title>

View File

@@ -0,0 +1,2 @@
<!DOCTYPE html>
<title>win_b</title>

View File

@@ -0,0 +1,52 @@
<!DOCTYPE html>
<head></head>
<body>
<script src="../testing.js"></script>
<!--
Navigating a browsing context in place (iframe.src change / popup re-nav) reuses
the same Window object, so a cached `iframe.contentWindow` / `window.open()`
return value keeps a VALID `.location` across the navigation. Previously the
in-place Frame re-init built a brand-new Window and freed the old one's
Location, so the cached reference dangled — reading `.location` re-wrapped the
freed Location into a double-free (URL.zig release-overflow).
Note: strict WindowProxy identity (`w === iframe.contentWindow` across the
navigation) is NOT yet provided — a Window is a v8 context's global object and
the context is recreated per navigation. Achieving that needs v8 global-proxy
reuse and is tracked separately. This test guards the UAF, not the identity.
-->
<script id=iframe_contentwindow_location_survives_navigation type=module>
const state = await testing.async();
const iframe = document.createElement('iframe');
const onceLoad = () => new Promise((r) => { iframe.onload = () => r(); });
let loaded = onceLoad();
iframe.src = 'support/win_a.html';
document.body.appendChild(iframe);
await loaded;
// Cache the window WITHOUT reading .location first, so its initial Location
// never gets a JS wrapper pinning it (the original crash condition).
const w = iframe.contentWindow;
loaded = onceLoad();
iframe.src = 'support/win_b.html'; // in-place re-init; Window is reused
await loaded;
state.resolve();
await state.done(() => {
// Reading the cached window's location returns the new, valid location
// (previously a UAF double-free on the freed Location).
testing.expectEqual(true, w.location.href.endsWith('/support/win_b.html'));
});
</script>
<!--
The popup re-navigation path (Session.processPopupNavigation) reuses the Window
the same way; it's exercised by the manual repro at wpt/_lprepro/opener.html
(real HTTP popup load + postMessage aren't driven by this htmlRunner harness).
-->
</body>