0

Ensure TreeWalker operations return Node in its own context, not the

receiver's

Normally, when Nodes are accessible across frames, we apply the
extended attribute [CheckSecurity=ReturnValue] in the IDL. The
generated bindings then perform the necessary security checks and
wrap the Node in its own context, not the receiver context.

TreeWalker is different. The root node is already known to be
accessible (otherwise it couldn't be passed to the constructor), and
therefore any Node returned by TreeWalker must be accessible, since
TreeWalker doesn't walk across frame boundaries. However, the
TreeWalker and the Node are not required to be from the same
context (I don't know why, it's an old API, this seems like the
kind of design flaw we would catch in a modern API). So TreeWalker
is in a weird middle ground: it doesn't need security checks, but
it does need to wrap any Nodes it returns in the Node's context,
not the TreeWalker's context.

This CL adds a new extended attribute, [NodeWrapInOwnContext], for
TreeWalker (and related APIs, NodeIterator and NodeFilter). When
the extended attribute is present, the Node will be wrapped for use
in V8 with the Node's v8::Context, not the TreeWalker's v8::Context.

Fixed: 324929076
Change-Id: I1359483a9b89f7fcc4d1d522a203e357f2136734
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5331177
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1352303}
This commit is contained in:
Nate Chapin
2024-09-06 21:36:29 +00:00
committed by Chromium LUCI CQ
parent ea2a975abb
commit baf4e5f8fb
8 changed files with 108 additions and 30 deletions

@ -1323,6 +1323,14 @@ Summary: Indicates that an IDL `Promise` type should be implemented as `v8::Loca
This is currently only used for the return types of `AsyncIteratorBase` methods. Consult with the bindings team before you use this extended attribute.
### [NodeWrapInOwnContext]
Summary: Forces a Node to be wrapped in its own context, rather than the receiver's context.
In most cases, return values are wrapped in the receiver context (i.e., the context of the interface whose operation/attribute/etc. is being called). The bindings assert that `[NodeWrapInOwnContext]` is only used for `Node`s. When used, we find the correct `ScriptState` with the `Node`'s `ExecutionContext` and the current `DOMWrapperWorld`, and if it exists, wrap the `Node` using that `ScriptState`. If that `ScriptState` is not available (usually because the `Node` is detached), we fall back to the receiver `ScriptState`.
`[NodeWrapInOwnContext]` is only necessary where a `Node` may be returned by an interface from a different context, *and* that interface does not use `[CheckSecurity=ReturnValue]` to enable cross-context security checks. The only interfaces that this applies to are ones that unnecessarily mix contexts (`NodeFilter`, `NodeIterator`, and `TreeWalker`), and new usage should not be introduced.
### [TargetOfExposed]
Summary: Interfaces specified with `[Global]` expose top-level IDL constructs specified with `[Exposed]` as JS data properties, however `[Global]` means a lot more (e.g. global object, named properties object, etc.). Interfaces specified with `[TargetOfExposed]` only expose top-level IDL constructs specified with `[Exposed]` and means nothing else.

@ -554,6 +554,14 @@ def make_blink_to_v8_value(
T = TextNode
F = FormatNode
if "NodeWrapInOwnContext" in idl_type.effective_annotations:
assert native_value_tag(idl_type, argument=argument) == "Node"
execution_context = blink_value_expr + "->GetExecutionContext()"
creation_context_script_state = _format(
"{_1} ? ToScriptState({_1}, {_2}->World()) : {_2}",
_1=execution_context,
_2=creation_context_script_state)
def create_definition(symbol_node):
binds = {
"blink_value_expr": blink_value_expr,

@ -1820,6 +1820,16 @@ def make_v8_set_return_value(cg_context):
#
# Note that the global object has its own context and there is no need to
# pass the creation context to ToV8.
null_context_body = [
T("""\
// Don't wrap the return value if its frame is in the process of detaching and
// has already invalidated its v8::Context, as it is not safe to
// re-initialize the v8::Context in that state. Return null instead.\
"""),
T("bindings::V8SetReturnValue(${info}, nullptr);"),
T("return;")
]
if (cg_context.member_like.extended_attributes.value_of("CheckSecurity") ==
"ReturnValue"):
node = CxxBlockNode([
@ -1830,22 +1840,14 @@ def make_v8_set_return_value(cg_context):
if cg_context.member_like.identifier == "frameElement" else
"${blink_receiver}->contentWindow()->GetFrame()"),
T("DCHECK(IsA<LocalFrame>(blink_frame));"),
CxxUnlikelyIfNode(
cond=T("!blink_frame->IsAttached() && "
"To<LocalFrame>(blink_frame)"
"->WindowProxyMaybeUninitialized("
"${script_state}->World())->ContextIfInitialized()"
".IsEmpty()"),
attribute="[[unlikely]]",
body=[
T("""\
// Don't wrap the return value if its frame is in the process of detaching and
// has already invalidated its v8::Context, as it is not safe to
// re-initialize the v8::Context in that state. Return null instead.\
"""),
T("bindings::V8SetReturnValue(${info}, nullptr);"),
T("return;")
]),
CxxUnlikelyIfNode(cond=T(
"!blink_frame->IsAttached() && "
"To<LocalFrame>(blink_frame)"
"->WindowProxyMaybeUninitialized("
"${script_state}->World())->ContextIfInitialized()"
".IsEmpty()"),
attribute="[[unlikely]]",
body=null_context_body),
F(
"v8::Local<v8::Value> v8_value = "
"ToV8Traits<{}>::ToV8("
@ -1860,6 +1862,24 @@ def make_v8_set_return_value(cg_context):
"third_party/blink/renderer/core/frame/local_frame.h",
]))
return node
if "NodeWrapInOwnContext" in cg_context.member_like.extended_attributes:
assert return_type.unwrap().identifier == "Node"
return CxxBlockNode([
T("ExecutionContext* node_execution_context = "
"${blink_receiver}->root()->GetExecutionContext();"),
T("ScriptState* node_script_state = node_execution_context ? "
"ToScriptState(node_execution_context, ${script_state}->World()) "
": ${script_state};"),
CxxUnlikelyIfNode(cond=T("!node_script_state"),
attribute="[[unlikely]]",
body=null_context_body),
T("// [NodeWrapInOwnContext]"),
F(
"v8::Local<v8::Value> v8_value = "
"ToV8Traits<{}>::ToV8(node_script_state, ${return_value});",
native_value_tag(return_type)),
T("bindings::V8SetReturnValue(${info}, v8_value);")
])
return_type = return_type.unwrap(typedef=True)
return_type_body = return_type.unwrap()

@ -154,6 +154,8 @@ def _build_supported_extended_attributes():
forms=F.IDENT),
E("NewObject", applicable_to=[T.OPERATION]),
E("NoAllocDirectCall", applicable_to=[T.OPERATION]),
E("NodeWrapInOwnContext",
applicable_to=[T.ATTRIBUTE, T.OPERATION, T.TYPE]),
E("NotEnumerable", applicable_to=[T.ATTRIBUTE, T.OPERATION]),
E("PassAsSpan", applicable_to=[T.TYPE]),
E("PermissiveDictionaryConversion", applicable_to=[T.DICTIONARY]),

@ -43,5 +43,5 @@
const unsigned long SHOW_DOCUMENT_FRAGMENT = 0x400;
const unsigned long SHOW_NOTATION = 0x800; // historical
unsigned short acceptNode(Node node);
unsigned short acceptNode([NodeWrapInOwnContext] Node node);
};

@ -23,14 +23,14 @@
[
Exposed=Window
] interface NodeIterator {
[SameObject] readonly attribute Node root;
readonly attribute Node referenceNode;
[NodeWrapInOwnContext, SameObject] readonly attribute Node root;
[NodeWrapInOwnContext] readonly attribute Node referenceNode;
readonly attribute boolean pointerBeforeReferenceNode;
readonly attribute unsigned long whatToShow;
readonly attribute NodeFilter? filter;
[RaisesException] Node? nextNode();
[RaisesException] Node? previousNode();
[NodeWrapInOwnContext, RaisesException] Node? nextNode();
[NodeWrapInOwnContext, RaisesException] Node? previousNode();
[MeasureAs=NodeIteratorDetach] void detach();
};

@ -23,16 +23,16 @@
[
Exposed=Window
] interface TreeWalker {
[SameObject] readonly attribute Node root;
[NodeWrapInOwnContext, SameObject] readonly attribute Node root;
readonly attribute unsigned long whatToShow;
readonly attribute NodeFilter? filter;
attribute Node currentNode;
[NodeWrapInOwnContext] attribute Node currentNode;
[RaisesException] Node? parentNode();
[RaisesException] Node? firstChild();
[RaisesException] Node? lastChild();
[RaisesException] Node? previousSibling();
[RaisesException] Node? nextSibling();
[RaisesException] Node? previousNode();
[RaisesException] Node? nextNode();
[NodeWrapInOwnContext, RaisesException] Node? parentNode();
[NodeWrapInOwnContext, RaisesException] Node? firstChild();
[NodeWrapInOwnContext, RaisesException] Node? lastChild();
[NodeWrapInOwnContext, RaisesException] Node? previousSibling();
[NodeWrapInOwnContext, RaisesException] Node? nextSibling();
[NodeWrapInOwnContext, RaisesException] Node? previousNode();
[NodeWrapInOwnContext, RaisesException] Node? nextNode();
};

@ -0,0 +1,40 @@
<!doctype html>
<title>TreeWalker tests</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<body>
<script>
test(t => {
const i = document.createElement("iframe");
i.srcdoc = "<!DOCTYPE html>";
document.body.appendChild(i);
t.add_cleanup(() => i.remove());
const walker = document.createTreeWalker(i.contentDocument,
NodeFilter.SHOW_ELEMENT);
walker.nextNode();
assert_true(walker.currentNode instanceof i.contentWindow.Node);
}, "Node returned by TreeWalker from different realm");
test(t => {
const i = document.createElement("iframe");
i.srcdoc = "<!DOCTYPE html>";
document.body.appendChild(i);
t.add_cleanup(() => i.remove());
let acceptNode_node;
const walker = document.createTreeWalker(
i.contentDocument, NodeFilter.SHOW_ELEMENT,
{
acceptNode(node) {
acceptNode_node = node;
return NodeFilter.FILTER_ACCEPT;
}
});
walker.nextNode();
assert_true(acceptNode_node instanceof i.contentWindow.Node);
assert_true(walker.currentNode instanceof i.contentWindow.Node);
}, "Node returned by TreeWalker from different realm with acceptNode");
</script>
</body>