Skip to content

Commit 58a0ade

Browse files
karlseguinkarlseguin
committed
Fix ce_reactions audit gaps and iterator-invalidation UAF
popAndInvoke captured the reactions queue as a slice at loop entry. If firing a reaction triggered a nested scope whose enqueue grew the underlying ArrayList, the captured slice became dangling and the next iteration read freed memory. Switch to indexed iteration so the queue pointer is re-read each step. Repros via wpt/custom-elements/ enqueue-custom-element-callback-reactions-inside-another-callback.html. The rest of the diff adds .ce_reactions = true to bridge declarations that were missing it per WebIDL [CEReactions] *and* whose Zig impl actually performs a DOM/attribute mutation (verified by reading each setter). Without the flag, the algorithm's enqueue path hits assertScopeActive and panics. Runtime-state-only setters (Input.value, Input.checked, Select.value, Option.selected, Media.muted/volume, etc.) are deliberately left untagged. Covered: - CustomElementRegistry.define, .upgrade - HTMLDocument: body, title, dir - Document: open, close - HTMLElement base: insertAdjacentHTML, dir, hidden, lang, tabIndex, title, innerText - Range: insertNode, deleteContents, extractContents, surroundContents, createContextualFragment - Selection: deleteFromDocument - HTMLOptionsCollection: add, remove - DOMStringMap (dataset): namedIndexed setter/deleter — required adding .ce_reactions to NamedIndexed.Opts in bridge.zig - ~28 HTMLxxxElement files: every settable attribute that resolves to setAttributeSafe/removeAttribute (Anchor, Button, Canvas, Data, Details, Dialog, FieldSet, Form, IFrame, Image, Input, Label, Link, LI, Media, Meta, OL, OptGroup, Option, Quote, Script, Select, Slot, Style, TableCell, Template, TextArea, Time, Track, Video)
1 parent 4f163c6 commit 58a0ade

40 files changed

Lines changed: 172 additions & 140 deletions

‎src/browser/CustomElementReactions.zig‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,12 @@ pub fn push(self: *Self) usize {
5959
/// enqueued within a nested scope drain at that scope's pop, before this loop
6060
/// sees them.
6161
pub fn popAndInvoke(self: *Self, checkpoint: usize, frame: *Frame) void {
62-
for (self.queue.items[checkpoint..]) |reaction| {
63-
Custom.fireReaction(reaction, frame);
62+
// Index, not slice: firing a reaction can recursively enqueue (via JS
63+
// callbacks doing DOM mutations), which may realloc queue.items and
64+
// invalidate any captured slice.
65+
var i = checkpoint;
66+
while (i < self.queue.items.len) : (i += 1) {
67+
Custom.fireReaction(self.queue.items[i], frame);
6468
}
6569
self.queue.items.len = checkpoint;
6670
self.active_scopes -= 1;

‎src/browser/js/bridge.zig‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,10 @@ pub const NamedIndexed = struct {
304304
const Opts = struct {
305305
as_typed_array: bool = false,
306306
null_as_undefined: bool = false,
307+
// Mirrors [CEReactions] on a named-property setter/deleter (e.g.,
308+
// HTMLElement.dataset, which proxies setAttribute/removeAttribute).
309+
// Only applies to setter and deleter; getters don't mutate.
310+
ce_reactions: bool = false,
307311
};
308312

309313
fn init(comptime T: type, comptime getter: anytype, setter: anytype, deleter: anytype, comptime opts: Opts) NamedIndexed {
@@ -332,6 +336,18 @@ pub const NamedIndexed = struct {
332336
}
333337
defer caller.deinit();
334338

339+
const ce_frame: ?*Frame = if (comptime opts.ce_reactions) switch (caller.local.ctx.global) {
340+
.frame => |frame| frame,
341+
.worker => null,
342+
} else null;
343+
var ce_checkpoint: usize = undefined;
344+
if (comptime opts.ce_reactions) {
345+
if (ce_frame) |frame| ce_checkpoint = frame._ce_reactions.push();
346+
}
347+
defer if (comptime opts.ce_reactions) {
348+
if (ce_frame) |frame| frame._ce_reactions.popAndInvoke(ce_checkpoint, frame);
349+
};
350+
335351
return caller.setNamedIndex(T, setter, c_name.?, c_value.?, handle.?, .{
336352
.as_typed_array = opts.as_typed_array,
337353
.null_as_undefined = opts.null_as_undefined,
@@ -348,6 +364,18 @@ pub const NamedIndexed = struct {
348364
}
349365
defer caller.deinit();
350366

367+
const ce_frame: ?*Frame = if (comptime opts.ce_reactions) switch (caller.local.ctx.global) {
368+
.frame => |frame| frame,
369+
.worker => null,
370+
} else null;
371+
var ce_checkpoint: usize = undefined;
372+
if (comptime opts.ce_reactions) {
373+
if (ce_frame) |frame| ce_checkpoint = frame._ce_reactions.push();
374+
}
375+
defer if (comptime opts.ce_reactions) {
376+
if (ce_frame) |frame| frame._ce_reactions.popAndInvoke(ce_checkpoint, frame);
377+
};
378+
351379
return caller.deleteNamedIndex(T, deleter, c_name.?, handle.?, .{
352380
.as_typed_array = opts.as_typed_array,
353381
.null_as_undefined = opts.null_as_undefined,

‎src/browser/webapi/CustomElementRegistry.zig‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,9 @@ pub const JsApi = struct {
260260
pub const prototype_chain = bridge.prototypeChain();
261261
};
262262

263-
pub const define = bridge.function(CustomElementRegistry.define, .{ .dom_exception = true });
263+
pub const define = bridge.function(CustomElementRegistry.define, .{ .dom_exception = true, .ce_reactions = true });
264264
pub const get = bridge.function(CustomElementRegistry.get, .{ .null_as_undefined = true });
265-
pub const upgrade = bridge.function(CustomElementRegistry.upgrade, .{});
265+
pub const upgrade = bridge.function(CustomElementRegistry.upgrade, .{ .ce_reactions = true });
266266
pub const whenDefined = bridge.function(CustomElementRegistry.whenDefined, .{ .dom_exception = true });
267267
};
268268

‎src/browser/webapi/Document.zig‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,8 +1171,8 @@ pub const JsApi = struct {
11711171
pub const elementsFromPoint = bridge.function(Document.elementsFromPoint, .{});
11721172
pub const write = bridge.function(Document.write, .{ .dom_exception = true, .ce_reactions = true });
11731173
pub const writeln = bridge.function(Document.writeln, .{ .dom_exception = true, .ce_reactions = true });
1174-
pub const open = bridge.function(Document.open, .{ .dom_exception = true });
1175-
pub const close = bridge.function(Document.close, .{ .dom_exception = true });
1174+
pub const open = bridge.function(Document.open, .{ .dom_exception = true, .ce_reactions = true });
1175+
pub const close = bridge.function(Document.close, .{ .dom_exception = true, .ce_reactions = true });
11761176
pub const doctype = bridge.accessor(Document.getDocType, null, .{});
11771177
pub const firstElementChild = bridge.accessor(Document.getFirstElementChild, null, .{});
11781178
pub const lastElementChild = bridge.accessor(Document.getLastElementChild, null, .{});

‎src/browser/webapi/HTMLDocument.zig‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,11 +288,11 @@ pub const JsApi = struct {
288288
});
289289
}
290290

291-
pub const dir = bridge.accessor(HTMLDocument.getDir, HTMLDocument.setDir, .{});
291+
pub const dir = bridge.accessor(HTMLDocument.getDir, HTMLDocument.setDir, .{ .ce_reactions = true });
292292
pub const head = bridge.accessor(HTMLDocument.getHead, null, .{});
293-
pub const body = bridge.accessor(HTMLDocument.getBody, HTMLDocument.setBody, .{ .dom_exception = true });
293+
pub const body = bridge.accessor(HTMLDocument.getBody, HTMLDocument.setBody, .{ .dom_exception = true, .ce_reactions = true });
294294
pub const lang = bridge.accessor(HTMLDocument.getLang, HTMLDocument.setLang, .{});
295-
pub const title = bridge.accessor(HTMLDocument.getTitle, HTMLDocument.setTitle, .{});
295+
pub const title = bridge.accessor(HTMLDocument.getTitle, HTMLDocument.setTitle, .{ .ce_reactions = true });
296296
pub const images = bridge.accessor(HTMLDocument.getImages, null, .{});
297297
pub const scripts = bridge.accessor(HTMLDocument.getScripts, null, .{});
298298
pub const links = bridge.accessor(HTMLDocument.getLinks, null, .{});

‎src/browser/webapi/Range.zig‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -718,12 +718,12 @@ pub const JsApi = struct {
718718
pub const isPointInRange = bridge.function(Range.isPointInRange, .{ .dom_exception = true });
719719
pub const intersectsNode = bridge.function(Range.intersectsNode, .{});
720720
pub const cloneRange = bridge.function(Range.cloneRange, .{ .dom_exception = true });
721-
pub const insertNode = bridge.function(Range.insertNode, .{ .dom_exception = true });
722-
pub const deleteContents = bridge.function(Range.deleteContents, .{ .dom_exception = true });
721+
pub const insertNode = bridge.function(Range.insertNode, .{ .dom_exception = true, .ce_reactions = true });
722+
pub const deleteContents = bridge.function(Range.deleteContents, .{ .dom_exception = true, .ce_reactions = true });
723723
pub const cloneContents = bridge.function(Range.cloneContents, .{ .dom_exception = true });
724-
pub const extractContents = bridge.function(Range.extractContents, .{ .dom_exception = true });
725-
pub const surroundContents = bridge.function(Range.surroundContents, .{ .dom_exception = true });
726-
pub const createContextualFragment = bridge.function(Range.createContextualFragment, .{ .dom_exception = true });
724+
pub const extractContents = bridge.function(Range.extractContents, .{ .dom_exception = true, .ce_reactions = true });
725+
pub const surroundContents = bridge.function(Range.surroundContents, .{ .dom_exception = true, .ce_reactions = true });
726+
pub const createContextualFragment = bridge.function(Range.createContextualFragment, .{ .dom_exception = true, .ce_reactions = true });
727727
pub const toString = bridge.function(Range.toString, .{ .dom_exception = true });
728728
pub const getBoundingClientRect = bridge.function(Range.getBoundingClientRect, .{});
729729
pub const getClientRects = bridge.function(Range.getClientRects, .{});

‎src/browser/webapi/Selection.zig‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ pub const JsApi = struct {
734734
pub const collapseToEnd = bridge.function(Selection.collapseToEnd, .{});
735735
pub const collapseToStart = bridge.function(Selection.collapseToStart, .{ .dom_exception = true });
736736
pub const containsNode = bridge.function(Selection.containsNode, .{});
737-
pub const deleteFromDocument = bridge.function(Selection.deleteFromDocument, .{});
737+
pub const deleteFromDocument = bridge.function(Selection.deleteFromDocument, .{ .ce_reactions = true });
738738
pub const empty = bridge.function(Selection.removeAllRanges, .{});
739739
pub const extend = bridge.function(Selection.extend, .{ .dom_exception = true });
740740
// unimplemented: getComposedRanges

‎src/browser/webapi/collections/HTMLOptionsCollection.zig‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,6 @@ pub const JsApi = struct {
106106
pub const @"[str]" = bridge.namedIndexed(HTMLOptionsCollection.getByName, null, null, .{ .null_as_undefined = true });
107107

108108
pub const selectedIndex = bridge.accessor(HTMLOptionsCollection.getSelectedIndex, HTMLOptionsCollection.setSelectedIndex, .{});
109-
pub const add = bridge.function(HTMLOptionsCollection.add, .{});
110-
pub const remove = bridge.function(HTMLOptionsCollection.remove, .{});
109+
pub const add = bridge.function(HTMLOptionsCollection.add, .{ .ce_reactions = true });
110+
pub const remove = bridge.function(HTMLOptionsCollection.remove, .{ .ce_reactions = true });
111111
};

‎src/browser/webapi/element/DOMStringMap.zig‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,5 +136,5 @@ pub const JsApi = struct {
136136
pub var class_id: bridge.ClassId = undefined;
137137
};
138138

139-
pub const @"[]" = bridge.namedIndexed(getProperty, setProperty, deleteProperty, .{ .null_as_undefined = true });
139+
pub const @"[]" = bridge.namedIndexed(getProperty, setProperty, deleteProperty, .{ .null_as_undefined = true, .ce_reactions = true });
140140
};

‎src/browser/webapi/element/Html.zig‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,21 +1234,21 @@ pub const JsApi = struct {
12341234

12351235
pub const constructor = bridge.constructor(HtmlElement.construct, .{ .new_target = true });
12361236

1237-
pub const innerText = bridge.accessor(_innerText, HtmlElement.setInnerText, .{});
1237+
pub const innerText = bridge.accessor(_innerText, HtmlElement.setInnerText, .{ .ce_reactions = true });
12381238
fn _innerText(self: *HtmlElement, frame: *const Frame) ![]const u8 {
12391239
var buf = std.Io.Writer.Allocating.init(frame.call_arena);
12401240
try self.getInnerText(&buf.writer);
12411241
return buf.written();
12421242
}
1243-
pub const insertAdjacentHTML = bridge.function(HtmlElement.insertAdjacentHTML, .{ .dom_exception = true });
1243+
pub const insertAdjacentHTML = bridge.function(HtmlElement.insertAdjacentHTML, .{ .dom_exception = true, .ce_reactions = true });
12441244
pub const click = bridge.function(HtmlElement.click, .{});
12451245

1246-
pub const dir = bridge.accessor(HtmlElement.getDir, HtmlElement.setDir, .{});
1247-
pub const hidden = bridge.accessor(HtmlElement.getHidden, HtmlElement.setHidden, .{});
1246+
pub const dir = bridge.accessor(HtmlElement.getDir, HtmlElement.setDir, .{ .ce_reactions = true });
1247+
pub const hidden = bridge.accessor(HtmlElement.getHidden, HtmlElement.setHidden, .{ .ce_reactions = true });
12481248
pub const isContentEditable = bridge.accessor(HtmlElement.getIsContentEditable, null, .{});
1249-
pub const lang = bridge.accessor(HtmlElement.getLang, HtmlElement.setLang, .{});
1250-
pub const tabIndex = bridge.accessor(HtmlElement.getTabIndex, HtmlElement.setTabIndex, .{});
1251-
pub const title = bridge.accessor(HtmlElement.getTitle, HtmlElement.setTitle, .{});
1249+
pub const lang = bridge.accessor(HtmlElement.getLang, HtmlElement.setLang, .{ .ce_reactions = true });
1250+
pub const tabIndex = bridge.accessor(HtmlElement.getTabIndex, HtmlElement.setTabIndex, .{ .ce_reactions = true });
1251+
pub const title = bridge.accessor(HtmlElement.getTitle, HtmlElement.setTitle, .{ .ce_reactions = true });
12521252

12531253
pub const onabort = bridge.accessor(HtmlElement.getOnAbort, HtmlElement.setOnAbort, .{});
12541254
pub const onanimationcancel = bridge.accessor(HtmlElement.getOnAnimationCancel, HtmlElement.setOnAnimationCancel, .{});

0 commit comments

Comments
 (0)