electron/patches/chromium/fix_harden_blink_scriptstate_maybefrom.patch
electron-roller[bot] ccd4531bfb
chore: bump chromium to 117.0.5852.0 (main) (#38891)
* chore: bump chromium in DEPS to 117.0.5846.0

* chore: update patches

* 4628901: Bump the macOS deployment target to 10.15

https://chromium-review.googlesource.com/c/chromium/src/+/4628901

* 4593350: [Private Network Access] Trigger Permission Prompt

https://chromium-review.googlesource.com/c/chromium/src/+/4593350

* 4631011: Remove unlaunched "InstallReplacementAndroidApp" Platform App APIs

https://chromium-review.googlesource.com/c/chromium/src/+/4631011

* chore: disable API deprecation warnings in NSKeyedArchiver

* chore: update libcxx filenames

* chore: bump chromium in DEPS to 117.0.5848.2

* chore: update feat_add_set_theme_source_to_allow_apps_to.patch

Xref: https://chromium-review.googlesource.com/c/chromium/src/+/4629743

No manual changes; patch succeeded with fuzz

* chore: update process_singleton.patch

Xref: https://chromium-review.googlesource.com/c/chromium/src/+/4605398

Trivial manual patch adjustments to account for code shear.

* chore: remove electron::BrowserContext::GetMediaDeviceIDSalt()

Xref: https://chromium-review.googlesource.com/c/chromium/src/+/4608130

upstream tldr:
- content::BrowserContext::GetMediaDeviceIDSalt()
- content::ContentBrowserClient::ArePersistentMediaDeviceIDsAllowed()
+ content::ContentBrowserClient::GetMediaDeviceIDSalt()

This commit leaves ElectronBrowserContext::GetMediaDeviceIDSalt() in
place (now non-virtual, non-override). It is now called by the new
function ElectronBrowserClient::GetMediaDeviceIDSalt().

As a followup, we might want to consider using the new upstream
media_device_salt::MediaDeviceSaltService and removing our
electron::MediaDeviceIDSalt code. CC @MarshallOfSound for 2nd
opinion since he has done the most work on MediaDeviceIDSalt and
may have more context.

* chore: fix iwyu breakage

Xref: https://chromium-review.googlesource.com/c/chromium/src/+/4629624

electron_browser_main_parts.cc uses ui::ColorProviderManager but didn't
include it. Things worked anyway because we got it indirectly from
content/public/browser/web_contents.h until 4629624.

* chore: remove call to base::mac::IsAtLeastOS10_14

upstream has bumped minimum version to 10.15 so this call is moot?

* chore: remove obsolete API_AVAILABLE calls in IAP

upstream has bumped minimum version to 10.15 so this call is moot?

* chore: remove obsolete API_AVAILABLE calls in electron_application_delegate

upstream has bumped minimum version to 10.15 so this call is moot?

* chore: remove broken-before-macOS-10.15 patch in mas_avoid_usage_of_private_macos_apis.patch

Upstream has bumped minimum to macOS 10.15

* chore: remove @available(macOS 10.14) check

Upstream minimum requirement for macOS is now 10.15

* chore: update patches

* chore: bump chromium in DEPS to 117.0.5850.0

* chore: update patches

* chore: bump chromium in DEPS to 117.0.5852.0

* chore: update patches

* Move two params from NetworkContextParams to NetworkContextFilePaths.

https://chromium-review.googlesource.com/c/chromium/src/+/4615930

* WebUSB: Add exclusionFilters to USBRequestDeviceOptions

https://chromium-review.googlesource.com/c/chromium/src/+/4614682

* Convert /chrome/browser/ui to use ARC

https://chromium-review.googlesource.com/c/chromium/src/+/4615920

* fixup! Bump the macOS deployment target to 10.15

* fixup! Bump the macOS deployment target to 10.15

* chore: update libcxx files

* win: Remove 10Glass from Windows10Glass function and var names

https://chromium-review.googlesource.com/c/chromium/src/+/4641314

* chore: revert 392e5f43 from chromium

* Add an ExecutionContext to ScriptState

https://chromium-review.googlesource.com/c/chromium/src/+/4609446

* fixup! Add an ExecutionContext to ScriptState

* chore: fix header

* Revert "chore: revert 392e5f43 from chromium"

This reverts commit b7f782943e4ce83cae8cd35780d8d3618cf0772c.

* fix: return correct min/max sizes in WinFrameView

* fixup! Revert chore: revert 392e5f43 from chromium

* fixup! Add an ExecutionContext to ScriptState

* Revert "fixup! Revert chore: revert 392e5f43 from chromium"

This reverts commit 7e2c7281abfc4f309255339fdba073d90a9ae3eb.

* Revert "fix: return correct min/max sizes in WinFrameView"

This reverts commit 3f418b1ab5155686730e087ae6cabe4a21b4bb61.

* Revert "Revert "chore: revert 392e5f43 from chromium""

This reverts commit 56296d8b7c434147e032e3c3b08c0e371b6c27ba.

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
Co-authored-by: deepak1556 <hop2deep@gmail.com>
Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
2023-07-01 16:22:55 -04:00

120 lines
5.3 KiB
Diff

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: deepak1556 <hop2deep@gmail.com>
Date: Wed, 28 Jun 2023 21:11:40 +0900
Subject: fix: harden blink::ScriptState::MaybeFrom
This is needed as side effect of https://chromium-review.googlesource.com/c/chromium/src/+/4609446
which now gets blink::ExecutionContext from blink::ScriptState
and there are isolate callbacks which get entered from Node.js
environment that has v8::Context not associated with blink::ScriptState.
Some examples are ModifyCodeGenerationFromStrings in node_bindings.cc,
blink::UseCounterCallback etc.
Without this patch when blink::ScriptState::MaybeFrom tries to extract
blink::ScriptState from the provided v8::Context and since Node.js has context
embedder data fields with index greater than blink (see node_context_data.h)
leading to the following CHECK failure.
```
script_state.h(169)] Security Check Failed: script_state
```
This patch adds a new tag in the context associated with ScriptState
to uniquely identify. It is based on what Node.js does to identify the
context created by it in `node_context_data.h`.
PS: We are not performing a check like
```
ScriptState* script_state =
static_cast<ScriptState*>(context->GetAlignedPointerFromEmbedderData(
kV8ContextPerContextDataIndex));
if (!script_state) {
return nullptr;
}
```
since in 32-bit builds which does not have v8 sandbox enabled unlike 64-bit builds,
the embedder data slot will not lazy initialize indexes in the former. This means
accessing uninitialized lower indexes can return garbage values that cannot be null checked.
Refer to v8::EmbedderDataSlot::store_aligned_pointer for context.
diff --git a/gin/public/gin_embedders.h b/gin/public/gin_embedders.h
index 8d7c5631fd8f1499c67384286f0e3c4037673b32..6a7491bc27334f6d1b1175eaa472c888e2b35b5e 100644
--- a/gin/public/gin_embedders.h
+++ b/gin/public/gin_embedders.h
@@ -18,6 +18,7 @@ namespace gin {
enum GinEmbedder : uint16_t {
kEmbedderNativeGin,
kEmbedderBlink,
+ kEmbedderBlinkTag,
kEmbedderPDFium,
kEmbedderFuchsia,
};
diff --git a/third_party/blink/renderer/platform/bindings/script_state.cc b/third_party/blink/renderer/platform/bindings/script_state.cc
index 7ff8785cd64c1264a88f91f7bd3292c6943f58ea..bc14ad8cab9fa3ec45bcb9f670b198970ecbeb92 100644
--- a/third_party/blink/renderer/platform/bindings/script_state.cc
+++ b/third_party/blink/renderer/platform/bindings/script_state.cc
@@ -13,6 +13,10 @@ namespace blink {
ScriptState::CreateCallback ScriptState::s_create_callback_ = nullptr;
+int const ScriptState::kScriptStateTag = 0x6e6f64;
+void* const ScriptState::kScriptStateTagPtr = const_cast<void*>(
+ static_cast<const void*>(&ScriptState::kScriptStateTag));
+
// static
void ScriptState::SetCreateCallback(CreateCallback create_callback) {
DCHECK(create_callback);
@@ -37,6 +41,8 @@ ScriptState::ScriptState(v8::Local<v8::Context> context,
DCHECK(world_);
context_.SetWeak(this, &OnV8ContextCollectedCallback);
context->SetAlignedPointerInEmbedderData(kV8ContextPerContextDataIndex, this);
+ context->SetAlignedPointerInEmbedderData(
+ kV8ContextPerContextDataTagIndex, ScriptState::kScriptStateTagPtr);
RendererResourceCoordinator::Get()->OnScriptStateCreated(this,
execution_context);
}
@@ -78,6 +84,8 @@ void ScriptState::DissociateContext() {
// Cut the reference from V8 context to ScriptState.
GetContext()->SetAlignedPointerInEmbedderData(kV8ContextPerContextDataIndex,
nullptr);
+ GetContext()->SetAlignedPointerInEmbedderData(
+ kV8ContextPerContextDataTagIndex, nullptr);
reference_from_v8_context_.Clear();
// Cut the reference from ScriptState to V8 context.
diff --git a/third_party/blink/renderer/platform/bindings/script_state.h b/third_party/blink/renderer/platform/bindings/script_state.h
index 7109852950cde0a6553000421faacefb39366b41..79be73cb660839d6074b11cd7491dc3d5e876345 100644
--- a/third_party/blink/renderer/platform/bindings/script_state.h
+++ b/third_party/blink/renderer/platform/bindings/script_state.h
@@ -178,7 +178,12 @@ class PLATFORM_EXPORT ScriptState : public GarbageCollected<ScriptState> {
static ScriptState* MaybeFrom(v8::Local<v8::Context> context) {
DCHECK(!context.IsEmpty());
if (context->GetNumberOfEmbedderDataFields() <=
- kV8ContextPerContextDataIndex) {
+ kV8ContextPerContextDataTagIndex) {
+ return nullptr;
+ }
+ if (context->GetAlignedPointerFromEmbedderData(
+ kV8ContextPerContextDataTagIndex) !=
+ ScriptState::kScriptStateTagPtr) {
return nullptr;
}
return From(context);
@@ -249,9 +254,15 @@ class PLATFORM_EXPORT ScriptState : public GarbageCollected<ScriptState> {
static void SetCreateCallback(CreateCallback);
friend class ScriptStateImpl;
+ static void* const kScriptStateTagPtr;
+ static int const kScriptStateTag;
static constexpr int kV8ContextPerContextDataIndex =
static_cast<int>(gin::kPerContextDataStartIndex) +
static_cast<int>(gin::kEmbedderBlink);
+ static constexpr int kV8ContextPerContextDataTagIndex =
+ static_cast<int>(gin::kPerContextDataStartIndex) +
+ static_cast<int>(gin::kEmbedderBlink) +
+ static_cast<int>(gin::kEmbedderBlinkTag);
};
// ScriptStateProtectingContext keeps the context associated with the