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.
This commit is contained in:
Karl Seguin
2026-07-11 07:20:58 +08:00
parent 897b446401
commit c2d4dbebc2
8 changed files with 76 additions and 22 deletions

View File

@@ -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 });

View File

@@ -49,4 +49,35 @@ reads of iframe data-* returned null (WPT encoding/*).
testing.expectEqual(null, iwin.readAttr('data-parentset'));
});
</script>
<script id="cross_realm_attr_node_identity" type=module>
{
const state = await testing.async();
const idf = document.getElementById('idf');
await new Promise((resolve) => {
if (idf.contentDocument.getElementById('s1')) return resolve();
idf.addEventListener('load', resolve);
});
const el = idf.contentDocument.getElementById('s1');
// Prime the Attr identity map from the parent, then have the iframe's own
// listener remove and re-add the attribute. The Attr identity map is keyed
// to the element's frame, so however the two sides' contexts resolve, the
// parent must see the re-added value, never a stale pre-delete wrapper.
const before = el.getAttributeNode('data-cycle');
const onmsg = (e) => {
if (e.data !== 'attr-cycled') return;
window.removeEventListener('message', onmsg);
state.resolve();
};
window.addEventListener('message', onmsg);
idf.contentWindow.postMessage('cycle-attr', '*');
await state.done(() => {
testing.expectEqual('A1B2', before.value);
testing.expectEqual('C3', el.getAttributeNode('data-cycle').value);
});
}
</script>
</body>

View File

@@ -1,10 +1,24 @@
<!DOCTYPE html>
<body>
<span id="s1" data-cp="5F" data-bytes="A1B2" nonstandardattr="ns1">x</span>
<span id="s1" data-cp="5F" data-bytes="A1B2" data-cycle="A1B2" nonstandardattr="ns1">x</span>
<script>
// Called by the parent test: reads execute in this frame's realm.
// Called by the parent test: these execute in this frame's realm.
function readAttr(name) {
return document.getElementById('s1').getAttribute(name);
}
function setAttr(name, value) {
document.getElementById('s1').setAttribute(name, value);
}
function removeAttr(name) {
document.getElementById('s1').removeAttribute(name);
}
window.addEventListener('message', (e) => {
if (e.data !== 'cycle-attr') return;
const el = document.getElementById('s1');
el.removeAttribute('data-cycle');
el.setAttribute('data-cycle', 'C3');
parent.postMessage('attr-cycled', '*');
});
</script>
</body>

View File

@@ -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,

View File

@@ -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 {

View File

@@ -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;

View File

@@ -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;

View File

@@ -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;