From c2d4dbebc2046db0217fc6cd9039206751e1ace0 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Sat, 11 Jul 2026 07:20:58 +0800 Subject: [PATCH] fix: When mutating attributes, use element's owner frame Currently, we use the calling frame when mutating an element's attributes (e.g. add/delete). This commit changes it to the element's owning frame. This should fix both some errors and potential uaf. --- src/browser/Frame.zig | 2 +- .../tests/frames/cross_realm_attributes.html | 31 ++++++++++++++++ .../support/cross_realm_attributes.html | 18 ++++++++- src/browser/webapi/Element.zig | 4 ++ src/browser/webapi/element/Attribute.zig | 37 +++++++++++-------- src/browser/webapi/element/html/Button.zig | 2 +- src/browser/webapi/element/html/Form.zig | 2 +- src/browser/webapi/element/html/Input.zig | 2 +- 8 files changed, 76 insertions(+), 22 deletions(-) diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index 23c2047c8..abe8642b9 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -3164,7 +3164,7 @@ pub fn submitForm(self: *Frame, submitter_: ?*Element, form_: ?*Element.Html.For const target_frame = blk: { const target_name = target_name_ orelse { - break :blk form_element.asNode().ownerFrame(self); + break :blk form_element.ownerFrame(self); }; break :blk self.resolveTargetFrame(target_name) orelse { log.warn(.not_implemented, "target", .{ .type = self._type, .url = self.url, .target = target_name }); diff --git a/src/browser/tests/frames/cross_realm_attributes.html b/src/browser/tests/frames/cross_realm_attributes.html index ac8722d0c..d06e5d45f 100644 --- a/src/browser/tests/frames/cross_realm_attributes.html +++ b/src/browser/tests/frames/cross_realm_attributes.html @@ -49,4 +49,35 @@ reads of iframe data-* returned null (WPT encoding/*). testing.expectEqual(null, iwin.readAttr('data-parentset')); }); + + diff --git a/src/browser/tests/frames/support/cross_realm_attributes.html b/src/browser/tests/frames/support/cross_realm_attributes.html index 7197b1cf2..756e5c0d4 100644 --- a/src/browser/tests/frames/support/cross_realm_attributes.html +++ b/src/browser/tests/frames/support/cross_realm_attributes.html @@ -1,10 +1,24 @@ - x + x diff --git a/src/browser/webapi/Element.zig b/src/browser/webapi/Element.zig index 2ed88adc4..65dc44bcb 100644 --- a/src/browser/webapi/Element.zig +++ b/src/browser/webapi/Element.zig @@ -1680,6 +1680,10 @@ pub fn getTag(self: *const Element) Tag { }; } +pub fn ownerFrame(self: *const Element, default: *Frame) *Frame { + return self._proto.ownerFrame(default); +} + pub const Tag = enum { address, anchor, diff --git a/src/browser/webapi/element/Attribute.zig b/src/browser/webapi/element/Attribute.zig index 111bb924c..8bc01915f 100644 --- a/src/browser/webapi/element/Attribute.zig +++ b/src/browser/webapi/element/Attribute.zig @@ -195,11 +195,13 @@ pub const List = struct { } // Identity map access: a given (list, name) always yields the same - // *Attribute until the attribute is removed. + // *Attribute until the attribute is removed. The map must be the + // element's frame's, not the caller's frame. pub fn getOrCreateAttribute(self: *const List, entry: *const Entry, element: ?*Element, frame: *Frame) !*Attribute { - const gop = try frame._attribute_lookup.getOrPut(frame.arena, .{ .list = self, .name = entry._name_ptr }); + const owner = if (element) |el| el.ownerFrame(frame) else frame; + const gop = try owner._attribute_lookup.getOrPut(owner.arena, .{ .list = self, .name = entry._name_ptr }); if (!gop.found_existing) { - gop.value_ptr.* = try entry.toAttribute(element, frame); + gop.value_ptr.* = try entry.toAttribute(element, owner); } return gop.value_ptr.*; } @@ -216,6 +218,7 @@ pub const List = struct { // The returned *Entry is only valid until the next mutation of the list. fn _put(self: *List, result: NormalizeAndEntry, value: String, element: *Element, frame: *Frame) !*Entry { + const owner = element.ownerFrame(frame); const is_id = shouldAddToIdMap(result.normalized, element); var entry: *Entry = undefined; @@ -224,16 +227,16 @@ pub const List = struct { // the old bytes are arena-owned or static; they outlive this update old_value = String.wrap(e.value()); if (is_id) { - frame.removeElementId(element, e.value()); + owner.removeElementId(element, e.value()); } - e.setValue(try frame.dupeString(value.str())); + e.setValue(try owner.dupeString(value.str())); entry = e; } else { - try self.ensureUnusedCapacity(1, frame); + try self.ensureUnusedCapacity(1, owner); entry = &self._entries[self._len]; entry.* = .init( - try canonicalizeName(result.normalized.str(), frame), - try frame.dupeString(value.str()), + try canonicalizeName(result.normalized.str(), owner), + try owner.dupeString(value.str()), ); self._len += 1; } @@ -242,10 +245,10 @@ pub const List = struct { const parent = element.asNode()._parent orelse { return entry; }; - try frame.addElementId(parent, element, entry.value()); + try owner.addElementId(parent, element, entry.value()); } - frame.domChanged(); - frame.attributeChange(element, result.normalized, .wrap(entry.value()), old_value); + owner.domChanged(); + owner.attributeChange(element, result.normalized, .wrap(entry.value()), old_value); return entry; } @@ -283,7 +286,8 @@ pub const List = struct { const entry = try self.put(attribute._name, attribute._value, element, frame); attribute._element = element; - try frame._attribute_lookup.put(frame.arena, .{ .list = self, .name = entry._name_ptr }, attribute); + const owner = element.ownerFrame(frame); + try owner._attribute_lookup.put(owner.arena, .{ .list = self, .name = entry._name_ptr }, attribute); return existing_attribute; } @@ -312,23 +316,24 @@ pub const List = struct { const result = try self.getEntryAndNormalizedName(name, frame); const entry = result.entry orelse return; + const owner = element.ownerFrame(frame); const is_id = shouldAddToIdMap(result.normalized, element); const old_value = entry.value(); if (is_id) { - frame.removeElementId(element, old_value); + owner.removeElementId(element, old_value); } // remove this BEFORE triggering anything, incase that re-enters delete // or some other callback. - _ = frame._attribute_lookup.remove(.{ .list = self, .name = entry._name_ptr }); + _ = owner._attribute_lookup.remove(.{ .list = self, .name = entry._name_ptr }); const index = (@intFromPtr(entry) - @intFromPtr(self._entries)) / @sizeOf(Entry); const list_entries = self._entries[0..self._len]; std.mem.copyForwards(Entry, list_entries[index .. list_entries.len - 1], list_entries[index + 1 ..]); self._len -= 1; - frame.domChanged(); - frame.attributeRemove(element, result.normalized, .wrap(old_value)); + owner.domChanged(); + owner.attributeRemove(element, result.normalized, .wrap(old_value)); } pub fn getNames(self: *const List, allocator: Allocator) ![][]const u8 { diff --git a/src/browser/webapi/element/html/Button.zig b/src/browser/webapi/element/html/Button.zig index bdf4f15fc..48426a5df 100644 --- a/src/browser/webapi/element/html/Button.zig +++ b/src/browser/webapi/element/html/Button.zig @@ -137,7 +137,7 @@ pub fn getLabels(self: *Button, frame: *Frame) !js.Array { pub fn getFormAction(self: *Button, frame: *Frame) ![]const u8 { const element = self.asElement(); - const owner_url = element.asNode().ownerFrame(frame).url; + const owner_url = element.ownerFrame(frame).url; const action = element.getAttributeSafe(comptime .wrap("formaction")) orelse return owner_url; if (action.len == 0) { return owner_url; diff --git a/src/browser/webapi/element/html/Form.zig b/src/browser/webapi/element/html/Form.zig index df9ae7512..7d0c6e220 100644 --- a/src/browser/webapi/element/html/Form.zig +++ b/src/browser/webapi/element/html/Form.zig @@ -115,7 +115,7 @@ pub fn iterator(self: *Form, frame: *Frame) collections.NodeLive(.form) { pub fn getAction(self: *Form, frame: *Frame) ![]const u8 { const element = self.asElement(); - const owner_url = element.asNode().ownerFrame(frame).url; + const owner_url = element.ownerFrame(frame).url; const action = element.getAttributeSafe(comptime .wrap("action")) orelse return owner_url; if (action.len == 0) { return owner_url; diff --git a/src/browser/webapi/element/html/Input.zig b/src/browser/webapi/element/html/Input.zig index 0524cd6e5..7731bf387 100644 --- a/src/browser/webapi/element/html/Input.zig +++ b/src/browser/webapi/element/html/Input.zig @@ -923,7 +923,7 @@ pub fn getForm(self: *Input, frame: *Frame) ?*Form { pub fn getFormAction(self: *Input, frame: *Frame) ![]const u8 { const element = self.asElement(); - const owner_url = element.asNode().ownerFrame(frame).url; + const owner_url = element.ownerFrame(frame).url; const action = element.getAttributeSafe(comptime .wrap("formaction")) orelse return owner_url; if (action.len == 0) { return owner_url;