Skip to content
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.

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

Already on GitHub? Sign in to your account

chore: cherry-pick 855df1837e from chromium #32033

Merged
merged 5 commits into from Jan 11, 2022

Conversation

ppontes
Copy link
Member

@ppontes ppontes commented Nov 27, 2021

sandbox: Fix sandbox inheritance [M96 merge]

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}

Release Notes

Notes: Backported fix for CVE-2021-38017.

@ppontes ppontes added security 馃敀 semver/patch backwards-compatible bug fixes backport-check-skip Skip trop's backport validity checking 14-x-y labels Nov 27, 2021
@ppontes ppontes requested a review from a team as a code owner November 27, 2021 16:49
@codebytere codebytere merged commit 8174553 into 14-x-y Jan 11, 2022
@codebytere codebytere deleted the cherry-pick/14-x-y/chromium/855df1837e branch January 11, 2022 15:27
@release-clerk
Copy link

release-clerk bot commented Jan 11, 2022

Release Notes Persisted

Backported fix for CVE-2021-38017.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
14-x-y backport-check-skip Skip trop's backport validity checking security 馃敀 semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants