From 3f72fb4a3c983de00bae9c8437a1c09df9c9955b Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Mon, 27 Feb 2017 11:43:55 -0800 Subject: Explicitly tag fxjs native objects. Native object callbacks have to distinguish whether the object they have been given is actually a native object and not some ordinary JS object. For method/property calls, this happens via v8's signature mechanism, but signature checks aren't applied to method arguments themselves. Currently, we do this by treating any object with an internal field count of 2 as being such, but this is fragile, and it has been pointed out that other objects with two internal fields are present. Additionally, that the first field points to a structure with a small zero-based object definition ID doesn't really have enough entropy to trust that it isn't some other entity. So add a pointer to an internal address in the second slot to make this safer. Note that we'll also get the same release_assert in the majority of cases as described in the bug. This is great from a security standpoint, but not great from a functional standpoint, except this likely only occurs in the wild if they are trying to mess with us. This just guards the theoretical cases that might pass the existing release_assert. BUG=695830 Change-Id: I42db27d6ed1143269a852805e4e4d862a8ab8773 Reviewed-on: https://pdfium-review.googlesource.com/2847 Commit-Queue: Tom Sepez Reviewed-by: dsinclair --- fxjs/fxjs_v8.cpp | 83 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 44 insertions(+), 39 deletions(-) diff --git a/fxjs/fxjs_v8.cpp b/fxjs/fxjs_v8.cpp index 0c211a130a..b0e1a1b260 100644 --- a/fxjs/fxjs_v8.cpp +++ b/fxjs/fxjs_v8.cpp @@ -20,12 +20,32 @@ static v8::Isolate* g_isolate = nullptr; static size_t g_isolate_ref_count = 0; static FXJS_ArrayBufferAllocator* g_arrayBufferAllocator = nullptr; static v8::Global* g_DefaultGlobalObjectTemplate = nullptr; +static wchar_t kPerObjectDataTag[] = L"CFXJS_PerObjectData"; class CFXJS_PerObjectData { public: explicit CFXJS_PerObjectData(int nObjDefID) : m_ObjDefID(nObjDefID), m_pPrivate(nullptr) {} + static void SetInObject(CFXJS_PerObjectData* pData, + v8::Local pObj) { + if (pObj->InternalFieldCount() == 2) { + pObj->SetAlignedPointerInInternalField(0, pData); + pObj->SetAlignedPointerInInternalField( + 1, static_cast(kPerObjectDataTag)); + } + } + + static CFXJS_PerObjectData* GetFromObject(v8::Local pObj) { + if (pObj.IsEmpty() || pObj->InternalFieldCount() != 2 || + pObj->GetAlignedPointerFromInternalField(1) != + static_cast(kPerObjectDataTag)) { + return nullptr; + } + return static_cast( + pObj->GetAlignedPointerFromInternalField(0)); + } + const int m_ObjDefID; void* m_pPrivate; }; @@ -241,13 +261,8 @@ CFXJS_Engine* CFXJS_Engine::CurrentEngineFromIsolate(v8::Isolate* pIsolate) { // static int CFXJS_Engine::GetObjDefnID(v8::Local pObj) { - if (pObj.IsEmpty() || !pObj->InternalFieldCount()) - return -1; - CFXJS_PerObjectData* pPerObjectData = static_cast( - pObj->GetAlignedPointerFromInternalField(0)); - if (!pPerObjectData) - return -1; - return pPerObjectData->m_ObjDefID; + CFXJS_PerObjectData* pData = CFXJS_PerObjectData::GetFromObject(pObj); + return pData ? pData->m_ObjDefID : -1; } // static @@ -257,10 +272,10 @@ void CFXJS_Engine::FreeObjectPrivate(void* pPerObjectData) { // static void CFXJS_Engine::FreeObjectPrivate(v8::Local pObj) { - if (pObj.IsEmpty() || !pObj->InternalFieldCount()) - return; - FreeObjectPrivate(pObj->GetAlignedPointerFromInternalField(0)); + CFXJS_PerObjectData* pData = CFXJS_PerObjectData::GetFromObject(pObj); pObj->SetAlignedPointerInInternalField(0, nullptr); + pObj->SetAlignedPointerInInternalField(1, nullptr); + delete pData; } int CFXJS_Engine::DefineObj(const char* sObjName, @@ -381,17 +396,17 @@ void CFXJS_Engine::InitializeEngine() { for (int i = 0; i < maxID; ++i) { CFXJS_ObjDefinition* pObjDef = CFXJS_ObjDefinition::ForID(m_isolate, i); if (pObjDef->m_ObjType == FXJSOBJTYPE_GLOBAL) { - v8Context->Global() - ->GetPrototype() - ->ToObject(v8Context) - .ToLocalChecked() - ->SetAlignedPointerInInternalField(0, new CFXJS_PerObjectData(i)); - - if (pObjDef->m_pConstructor) + CFXJS_PerObjectData::SetInObject(new CFXJS_PerObjectData(i), + v8Context->Global() + ->GetPrototype() + ->ToObject(v8Context) + .ToLocalChecked()); + if (pObjDef->m_pConstructor) { pObjDef->m_pConstructor(this, v8Context->Global() ->GetPrototype() ->ToObject(v8Context) .ToLocalChecked()); + } } else if (pObjDef->m_ObjType == FXJSOBJTYPE_STATIC) { v8::Local pObjName = v8::String::NewFromUtf8(m_isolate, pObjDef->m_ObjName, @@ -500,15 +515,14 @@ v8::Local CFXJS_Engine::NewFxDynamicObj(int nObjDefnID, if (!pObjDef->GetInstanceTemplate()->NewInstance(context).ToLocal(&obj)) return v8::Local(); - CFXJS_PerObjectData* pPerObjData = new CFXJS_PerObjectData(nObjDefnID); - obj->SetAlignedPointerInInternalField(0, pPerObjData); + CFXJS_PerObjectData* pObjData = new CFXJS_PerObjectData(nObjDefnID); + CFXJS_PerObjectData::SetInObject(pObjData, obj); if (pObjDef->m_pConstructor) pObjDef->m_pConstructor(this, obj); - if (!bStatic && FXJS_PerIsolateData::Get(m_isolate)->m_pDynamicObjsMap) { - FXJS_PerIsolateData::Get(m_isolate)->m_pDynamicObjsMap->set(pPerObjData, - obj); - } + if (!bStatic && FXJS_PerIsolateData::Get(m_isolate)->m_pDynamicObjsMap) + FXJS_PerIsolateData::Get(m_isolate)->m_pDynamicObjsMap->set(pObjData, obj); + return obj; } @@ -534,34 +548,25 @@ void CFXJS_Engine::Error(const CFX_WideString& message) { } void CFXJS_Engine::SetObjectPrivate(v8::Local pObj, void* p) { - if (pObj.IsEmpty() || !pObj->InternalFieldCount()) - return; - CFXJS_PerObjectData* pPerObjectData = static_cast( - pObj->GetAlignedPointerFromInternalField(0)); + CFXJS_PerObjectData* pPerObjectData = + CFXJS_PerObjectData::GetFromObject(pObj); if (!pPerObjectData) return; pPerObjectData->m_pPrivate = p; } void* CFXJS_Engine::GetObjectPrivate(v8::Local pObj) { - if (pObj.IsEmpty()) - return nullptr; - CFXJS_PerObjectData* pPerObjectData = nullptr; - if (pObj->InternalFieldCount()) { - pPerObjectData = static_cast( - pObj->GetAlignedPointerFromInternalField(0)); - } else { + CFXJS_PerObjectData* pData = CFXJS_PerObjectData::GetFromObject(pObj); + if (!pData && !pObj.IsEmpty()) { // It could be a global proxy object. v8::Local v = pObj->GetPrototype(); v8::Local context = m_isolate->GetCurrentContext(); if (v->IsObject()) { - pPerObjectData = static_cast( - v->ToObject(context) - .ToLocalChecked() - ->GetAlignedPointerFromInternalField(0)); + pData = CFXJS_PerObjectData::GetFromObject( + v->ToObject(context).ToLocalChecked()); } } - return pPerObjectData ? pPerObjectData->m_pPrivate : nullptr; + return pData ? pData->m_pPrivate : nullptr; } v8::Local CFXJS_Engine::GetObjectProperty( -- cgit v1.2.3