From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: deepak1556 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(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 6d11f4401d50476802e514392615d287fa54522e..262aca59159985b4f6359f7f9d6763353124515b 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, }; enum EmbedderDataTag : uint16_t { diff --git a/third_party/blink/renderer/platform/bindings/script_state.cc b/third_party/blink/renderer/platform/bindings/script_state.cc index c7e892691cbec1c2e8e97f6972c2b8374a166167..332c7a78860776d7a0e3ed911ca50e1c664f18dc 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( + static_cast(&ScriptState::kScriptStateTag)); + // static void ScriptState::SetCreateCallback(CreateCallback create_callback) { DCHECK(create_callback); @@ -38,6 +42,8 @@ ScriptState::ScriptState(v8::Local context, context_.SetWeak(this, &OnV8ContextCollectedCallback); context->SetAlignedPointerInEmbedderData(kV8ContextPerContextDataIndex, this, gin::kBlinkScriptState); + context->SetAlignedPointerInEmbedderData( + kV8ContextPerContextDataTagIndex, ScriptState::kScriptStateTagPtr); RendererResourceCoordinator::Get()->OnScriptStateCreated(this, execution_context); } @@ -81,6 +87,8 @@ void ScriptState::DissociateContext() { // Cut the reference from V8 context to ScriptState. GetContext()->SetAlignedPointerInEmbedderData( kV8ContextPerContextDataIndex, nullptr, gin::kBlinkScriptState); + 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 f06885f429a395b5c2eb55c89803837b550d765c..3340e4ec8d1ea20ea8310f288428b5869e85392a 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 { v8::Local 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 { static void SetCreateCallback(CreateCallback); friend class ScriptStateImpl; + static void* const kScriptStateTagPtr; + static int const kScriptStateTag; static constexpr int kV8ContextPerContextDataIndex = static_cast(gin::kPerContextDataStartIndex) + static_cast(gin::kEmbedderBlink); @@ -271,6 +278,10 @@ class PLATFORM_EXPORT ScriptState : public GarbageCollected { // internals.idl. String last_compiled_script_file_name_; bool last_compiled_script_used_code_cache_ = false; + + static constexpr int kV8ContextPerContextDataTagIndex = + static_cast(gin::kPerContextDataStartIndex) + + static_cast(gin::kEmbedderBlinkTag); }; // ScriptStateProtectingContext keeps the context associated with the