electron/patches/chromium/fix_harden_blink_scriptstate_maybefrom.patch
electron-roller[bot] dd03cceda0
chore: bump chromium to 137.0.7128.1 (main) (#46482)
* chore: bump chromium in DEPS to 137.0.7107.0

* chore: bump chromium in DEPS to 137.0.7109.0

* chore: bump chromium in DEPS to 137.0.7111.0

* chore: bump chromium in DEPS to 137.0.7113.0

* 6384240: Remove double-declaration for accessibility on macOS | 6384240

* 6422872: Remove unused includes in isolation_info_mojom_traits.h | 6422872

* chore: update patches

* 6400733: Avoid ipc_message_macros.h usage in some foo_param_traits_macros.h files | 6400733

* chore: update patches

* 6423410: Enable unsafe buffer warnings for chromium, try #3. | 6423410

* chore: iwyu

* refactor: prefer value initialization over memset()

From the looks up upstream commits in base/, it looks like memset()
could trigger `-Wunsafe-buffer-usage` warnings soon?

Value initialization is more C++ish and less error-prone anyway,
due to memset()'s easily swappable parameters.

* refactor: NotifyIcon::InitIconData() returns a NOTIFYICONDATA

This follows F.20 in the C++ Core Guidelines and also removes the need
for memset()

* 6423410: Enable unsafe buffer warnings for chromium, try #3. | 6423410

remove all uses of:

- strcmp()

* fixup!  6423410: Enable unsafe buffer warnings for chromium, try #3. | 6423410

* 6433203: Add a PassKey to RegisterDeleteDelegateCallback(). | 6433203

* chore: bump chromium in DEPS to 137.0.7115.0

* 6387077: [PermissionOptions] Generalize PermissionRequestDescription | 6387077

* chore: update patches

* 6387077: [PermissionOptions] Generalize PermissionRequestDescription | 6387077

* fix: add pragma for MacSDK unsafe buffers | 6423410: Enable unsafe buffer warnings for chromium, try #3. | 6423410

* chore: bump chromium in DEPS to 137.0.7117.0

* chore: update patches

* chore: update filesnames.libcxx.gni

* 6431756: Replace SetOwnedByWidget() bool arg with a PassKey. | 6431756

* 6387077: [PermissionOptions] Generalize PermissionRequestDescription | 6387077

* 6428345: Remove ExtensionService usage from ChromeExtensionRegistrarDelegate | 6428345

* 6384315: Migrate extensions_enabled from ExtensionService to Registrar | 6384315

* 6428749: [extensions] Refactor ExtensionService for AddNewAndUpdateExtension. | 6428749

* chore: bump chromium in DEPS to 137.0.7119.0

* 6440290: corner-shape: support inset shadow | 6440290

* 6429230: FSA: Move blocked paths to the PermissionContext class | 6429230

* chore: update patches

* chore: bump chromium in DEPS to 137.0.7121.0

* chore: update patches

* fix: partially revert 6443473: Remove ItemDelete from the Mac version of AppleKeychain | 6443473

* fix: update filenames.libcxx.gni

* chore: bump chromium in DEPS to 137.0.7123.0

* chore: update patches

* chore: "grandfather in" electron views too

Lock further access to View::set_owned_by_client() | 6448510

* chore: update feat_corner_smoothing_css_rule_and_blink_painting.patch

corner-shape: support inset shadow | 6440290

* refactor: grandfather in AutofillPopupView as a subclass of WidgetDelegateView

Add a PassKey for std::make_unique<WidgetDelegateView>() | 6442265

* Provide dbus appmenu information on Wayland | 6405535

* [extensions] Move OnExtensionInstalled out of ExtensionService. | 6443325

* refactor: grandfather in NativeWindowViews for delete callbacks

6433203: Add a PassKey to RegisterDeleteDelegateCallback(). | 6433203

* chore: merge the four "grandfather" patches into one

* [A11yPerformance] Remove IsAccessibilityAllowed() | 6404386: [A11yPerformance] Remove IsAccessibilityAllowed() | 6404386

NB: the changes here are copied from the upstream changes in
chrome/browser/ui/webui/accessibility/accessibility_ui.cc

* 6420753: [PermissionOptions] Use PermissionDescriptorPtr in PermissionController | 6420753

* 6429573: [accessibility] Move mode change out of AccessibilityNotificationWaiter | 6429573

* chore: e patches all

* 6419936: [win] Change ScreenWin public static methods to virtual | 6419936

* 6423410: Enable unsafe buffer warnings for chromium, try #3. | 6423410

remove all uses of:

- fprintf()
- fputs()
- snprintf()
- vsnprintf()

* fix: size conversion FTBFS on Win

* 6423410: Enable unsafe buffer warnings for chromium, try #3. | 6423410

remove all uses of:

- wcscpy_s()

* 6423410: Enable unsafe buffer warnings for chromium, try #3. | 6423410

remove all uses of:

- wcsncpy_s()

* chore: update mas_avoid_private_macos_api_usage.patch.patch

6394283: Remove double-declaration for accessibility on iOS | 6394283

Lots of context shear in this commit but the only interesting part is:

-+  return nullptr;
++  return {};

Which is needed because the return type is sometimes not a pointer.

* chore: e patches all

* chore: disable -Wmacro-redefined warning in electron_main_win.cc

* chore: bump chromium in DEPS to 137.0.7123.5

* refactor: patch electron PermissionTypes into blink

6387077: [PermissionOptions] Generalize PermissionRequestDescription | 6387077

* chore: e patches all

* chore: remove the box_painter_base.cc part of feat_corner_smoothing_css_rule_and_blink_painting.patch

as per code review @ https://github.com/electron/electron/pull/46482#pullrequestreview-2777338370

* test: enable window-smaller-than-64x64 test on Linux

* chore: bump chromium in DEPS to 137.0.7124.1

* chore: bump chromium in DEPS to 137.0.7125.1

* chore: bump chromium in DEPS to 137.0.7127.3

* 6459201: [Extensions] Remove ExtensionSystem::FinishDelayedInstallationIfReady() | 6459201

* 6454796: [Extensions] Move (most) registrar delayed install logic to //extensions | 6454796

* chore: bump chromium in DEPS to 137.0.7128.1

* chore: e patches all

* chore: node ./script/gen-libc++-filenames.js

* [views] Gate DesktopWindowTreeHostWin::window_enlargement_ behind flag

Refs 6428649

* feat: allow opt-out animated_content_sampler.

Refs 6438681

* Trigger CI

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Keeley Hammond <khammond@slack-corp.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
Co-authored-by: Keeley Hammond <vertedinde@electronjs.org>
Co-authored-by: deepak1556 <hop2deep@gmail.com>
Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
2025-04-22 15:53:29 -04:00

125 lines
5.5 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..2b7bdfbac06a42e6bc51eb65e023c3673e6eb885 100644
--- a/gin/public/gin_embedders.h
+++ b/gin/public/gin_embedders.h
@@ -20,6 +20,8 @@ enum GinEmbedder : uint16_t {
kEmbedderBlink,
kEmbedderPDFium,
kEmbedderFuchsia,
+ kEmbedderElectron,
+ kEmbedderBlinkTag,
};
} // namespace gin
diff --git a/third_party/blink/renderer/platform/bindings/script_state.cc b/third_party/blink/renderer/platform/bindings/script_state.cc
index 58aa3d5b24355a4346c9d6992e6090d28cad513f..96c28c190f98c76c0defeaabda092ebdf6fb44c6 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);
}
@@ -80,6 +86,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 b3cc8d819b06108386aed9465cab4f27a28b675f..9c8818f10de59fdd2a3fd44d9cd23d40a93b53a7 100644
--- a/third_party/blink/renderer/platform/bindings/script_state.h
+++ b/third_party/blink/renderer/platform/bindings/script_state.h
@@ -185,7 +185,12 @@ class PLATFORM_EXPORT ScriptState : public GarbageCollected<ScriptState> {
v8::Local<v8::Context> context) {
DCHECK(!context.IsEmpty());
if (context->GetNumberOfEmbedderDataFields() <=
- kV8ContextPerContextDataIndex) {
+ kV8ContextPerContextDataTagIndex) {
+ return nullptr;
+ }
+ if (context->GetAlignedPointerFromEmbedderData(
+ kV8ContextPerContextDataTagIndex) !=
+ ScriptState::kScriptStateTagPtr) {
return nullptr;
}
ScriptState* script_state =
@@ -263,6 +268,8 @@ 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);
@@ -271,6 +278,10 @@ class PLATFORM_EXPORT ScriptState : public GarbageCollected<ScriptState> {
// internals.idl.
String last_compiled_script_file_name_;
bool last_compiled_script_used_code_cache_ = false;
+
+ static constexpr int kV8ContextPerContextDataTagIndex =
+ static_cast<int>(gin::kPerContextDataStartIndex) +
+ static_cast<int>(gin::kEmbedderBlinkTag);
};
// ScriptStateProtectingContext keeps the context associated with the