8000 chore: cherry-pick 855df1837e from chromium by ppontes · Pull Request #32034 · electron/electron · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

chore: cherry-pick 855df1837e from chromium #32034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 11, 2022
Merged

chore: cherry-pick 855df1837e from chromium #32034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
efde64c
chore: cherry-pick 855df1837e from chromium
ppontes Nov 27, 2021
df8f747
Merge branch '13-x-y' into cherry-pick/13-x-y/chromium/855df1837e
electron-bot Jan 4, 2022
f6d58a6
Merge branch '13-x-y' into cherry-pick/13-x-y/chromium/855df1837e
electron-bot Jan 10, 2022
b1d5e14
Merge branch '13-x-y' into cherry-pick/13-x-y/chromium/855df1837e
electron-bot Jan 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/chromium/.patches
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ allow_null_skbitmap_to_be_transferred_across_threads.patch
use_weakptrs_for_the_threadediconloader_s_background_tasks.patch
cachestorage_store_partial_opaque_responses.patch
cherry-pick-a5f54612590d.patch
sandbox_fix_sandbox_inheritance_m96_merge.patch
cherry-pick-dbde8795233a.patch
cherry-pick-6bb320d134b1.patch
m96_fileapi_move_origin_checks_in_bloburlstore_sooner.patch
153 changes: 153 additions & 0 deletions patches/chromium/sandbox_fix_sandbox_inheritance_m96_merge.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Wed, 3 Nov 2021 12:49:50 +0000
Subject: sandbox: Fix sandbox inheritance [M96 merge]
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is a cherry-pick of the following patch:
https://chromium-review.googlesource.com/c/chromium/src/+/3231298

Patch description:
------------------
When creating a new frame and the initial empty document, blink was only
sending the frame's sandbox attribute, but without combining with its owner's
(=document) sandbox flags.

This patch combines frame's attribute with its document sandbox flags.

🎁 Arthur Sonzogni wishes for a better future: 🎁
-------------------------------------------------
There are no good reasons sandbox flags inheritance to be complicated.
See: content/browser/renderer_host/sandbox_flags.md

For legacy reasons, Chrome's developers were confused about what objects
have frame or document semantic. On the browser process, the
FrameTreeNode represents the frame and the RenderFrameHost is almost
(%RenderDocument) the document/execution-context.

Currently, sandbox flags is plumbed inside FramePolicy, and it is not
clear to me whether FramePolicy is a frame-scoped or document-scoped
property (or both).
The current logic speak about "pending" FramePolicy (=frame) and
"active" FramePolicy (=document) and store both type into the
FrameTreeNode and RenderFrameHost, which is not ideal.

I believe we should extract SandboxFlags outside of FramePolicy and
make a very clean implementation, parallel to the PolicyContainer logic.
In a second step it could also be integrated into PolicyContainer, if we
resolve the additional property that sandbox flags can also be further
restricted at the frame level, similar to CSP embedded enforcement.

Bug: 1256822, 1262061
Change-Id: Id38de6d7eeeb1e4fa7722ab56288666763fae838
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3231298
Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Antonio Sartori <antoniosartori@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#933845}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3256558
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/branch-heads/4664@{#695}
Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512}

diff --git a/content/browser/bad_message.h b/content/browser/bad_message.h
index d1821c863d998cb010d8f5d12c12d79b18075a9d..79498c6c815bfc08d6d543b7302dc981b71580f3 100644
--- a/content/browser/bad_message.h
+++ b/content/browser/bad_message.h
@@ -271,6 +271,9 @@ enum BadMessageReason {
WCI_INVALID_DOWNLOAD_IMAGE_RESULT = 243,
FARI_LOGOUT_BAD_ENDPOINT = 250,
RFH_CHILD_FRAME_UNEXPECTED_OWNER_ELEMENT_TYPE = 251,
+ RFH_POPUP_REQUEST_WHILE_PRERENDERING = 252,
+ RFH_INTERECEPT_DOWNLOAD_WHILE_INACTIVE = 253, // Unused until 97.0.4674.0
+ RFH_CREATE_CHILD_FRAME_SANDBOX_FLAGS = 254,

// Please add new elements here. The naming convention is abbreviated class
// name (e.g. RenderFrameHost becomes RFH) plus a unique description of the
diff --git a/content/browser/renderer_host/frame_tree_node.cc b/content/browser/renderer_host/frame_tree_node.cc
index a57e16e97f7871c5a4d34a33e87cf372f228b444..155f570b7e011237b4ea505f5d17e19ee502e92a 100644
--- a/content/browser/renderer_host/frame_tree_node.cc
+++ b/content/browser/renderer_host/frame_tree_node.cc
@@ -469,6 +469,12 @@ bool FrameTreeNode::HasPendingCrossDocumentNavigation() const {

bool FrameTreeNode::CommitFramePolicy(
const blink::FramePolicy& new_frame_policy) {
+ // Documents create iframes, iframes host new documents. Both are associated
+ // with sandbox flags. They are required to be stricter or equal to their
+ // owner when they change, as we go down.
+ // TODO(https://crbug.com/1262061). Enforce the invariant mentioned above,
+ // once the interactions with FencedIframe has been tested and clarified.
+
bool did_change_flags = new_frame_policy.sandbox_flags !=
replication_state_->frame_policy.sandbox_flags;
bool did_change_container_policy =
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index db01f3ea0423d780763ba82e50725bb0a12e5018..d3dfaaed9157bfbe7ffcb146774bd7323d56ae06 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -2836,6 +2836,16 @@ void RenderFrameHostImpl::CreateChildFrame(
return;
}

+ // Documents create iframes, iframes host new documents. Both are associated
+ // with sandbox flags. They are required to be stricter or equal to their
+ // owner when they are created, as we go down.
+ if (frame_policy.sandbox_flags !=
+ (frame_policy.sandbox_flags | active_sandbox_flags())) {
+ bad_message::ReceivedBadMessage(
+ GetProcess(), bad_message::RFH_CREATE_CHILD_FRAME_SANDBOX_FLAGS);
+ return;
+ }
+
// TODO(crbug.com/1145708). The interface exposed to tests should
// match the mojo interface.
OnCreateChildFrame(new_routing_id, std::move(frame_remote),
diff --git a/third_party/blink/renderer/core/frame/web_local_frame_impl.cc b/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
index 8660df6074865b845f1b8341cb1348adb28c2339..7a420c241491a9780fdd5466973ff70a97800744 100644
--- a/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
+++ b/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
@@ -2069,6 +2069,16 @@ LocalFrame* WebLocalFrameImpl::CreateChildFrame(
policy_container_receiver =
policy_container_remote.InitWithNewEndpointAndPassReceiver();

+ FramePolicy frame_policy = owner_element->GetFramePolicy();
+ // Documents create iframes, iframes host new documents. Both are associated
+ // with sandbox flags. They are required to be stricter or equal as we go
+ // down. The iframe owner element only returns the additional restrictions
+ // defined in the HTMLIFrameElement's sanbox attribute. It needs to be
+ // combined with the document's sandbox flags to get the frame's sandbox
+ // policy right.
+ frame_policy.sandbox_flags |=
+ GetFrame()->GetDocument()->GetExecutionContext()->GetSandboxFlags();
+
// FIXME: Using subResourceAttributeName as fallback is not a perfect
// solution. subResourceAttributeName returns just one attribute name. The
// element might not have the attribute, and there might be other attributes
@@ -2078,8 +2088,7 @@ LocalFrame* WebLocalFrameImpl::CreateChildFrame(
scope, name,
owner_element->getAttribute(
owner_element->SubResourceAttributeName()),
- owner_element->GetFramePolicy(), owner_properties,
- owner_element->OwnerType(),
+ std::move(frame_policy), owner_properties, owner_element->OwnerType(),
WebPolicyContainerBindParams{std::move(policy_container_receiver)}));
if (!webframe_child)
return nullptr;
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index d6dd81a4af0fb71b0ab6b410bfb20737dfdbdbf9..275892d5cb3a9af8fdbe1ed81169398367fa30b1 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -7226,6 +7226,9 @@ Called by update_bad_message_reasons.py.-->
<int value="243" label="WCI_INVALID_DOWNLOAD_IMAGE_RESULT"/>
<int value="250" label="FARI_LOGOUT_BAD_ENDPOINT"/>
<int value="251" label="RFH_CHILD_FRAME_UNEXPECTED_OWNER_ELEMENT_TYPE"/>
+ <int value="252" label="RFH_POPUP_REQUEST_WHILE_PRERENDERING"/>
+ <int value="253" label="RFH_INTERECEPT_DOWNLOAD_WHILE_INACTIVE"/>
+ <int value="254" label="RFH_CREATE_CHILD_FRAME_SANDBOX_FLAGS"/>
</enum>

<enum name="BadMessageReasonExtensions">
0