From d86805176f390e0fec1802aae7dbbf1d1d9f53b0 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 24 Oct 2018 00:15:53 +0000 Subject: Be more particular about FX objects constructed from JS This is back-filling some more error cases from the work from a few weeks ago. Replaces a lambda with a static CallHandler() method since the verbosity was increasing. It gets invoked if you try to make a new FXJS object from the javascript side, rather than the C++ side. Making such an object is a little tricky, since we don't give these functions names in V8, but they can be obtained via constructor property from an instance of the object. Change-Id: Ibca686e75338ac54d08a114f36f930cd424a1eb5 Reviewed-on: https://pdfium-review.googlesource.com/c/44534 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang --- fxjs/cfxjs_engine.cpp | 32 ++++++--- testing/resources/javascript/bug_695826.in | 7 +- testing/resources/javascript/constructor.in | 77 ++++++++++++++++++++++ .../resources/javascript/constructor_expected.txt | 32 +++++++++ 4 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 testing/resources/javascript/constructor.in create mode 100644 testing/resources/javascript/constructor_expected.txt diff --git a/fxjs/cfxjs_engine.cpp b/fxjs/cfxjs_engine.cpp index c993a7dafa..d329e13205 100644 --- a/fxjs/cfxjs_engine.cpp +++ b/fxjs/cfxjs_engine.cpp @@ -126,15 +126,9 @@ class CFXJS_ObjDefinition { m_pIsolate(isolate) { v8::Isolate::Scope isolate_scope(isolate); v8::HandleScope handle_scope(isolate); - v8::Local fun = v8::FunctionTemplate::New(isolate); fun->InstanceTemplate()->SetInternalFieldCount(2); - fun->SetCallHandler([](const v8::FunctionCallbackInfo& info) { - v8::Local holder = info.Holder(); - ASSERT(holder->InternalFieldCount() == 2); - holder->SetAlignedPointerInInternalField(0, nullptr); - holder->SetAlignedPointerInInternalField(1, nullptr); - }); + fun->SetCallHandler(CallHandler, v8::Number::New(isolate, eObjType)); if (eObjType == FXJSOBJTYPE_GLOBAL) { fun->InstanceTemplate()->Set( v8::Symbol::GetToStringTag(isolate), @@ -142,9 +136,29 @@ class CFXJS_ObjDefinition { .ToLocalChecked()); } m_FunctionTemplate.Reset(isolate, fun); + m_Signature.Reset(isolate, v8::Signature::New(isolate, fun)); + } - v8::Local sig = v8::Signature::New(isolate, fun); - m_Signature.Reset(isolate, sig); + static void CallHandler(const v8::FunctionCallbackInfo& info) { + v8::Isolate* isolate = info.GetIsolate(); + if (!info.IsConstructCall()) { + isolate->ThrowException( + v8::String::NewFromUtf8(isolate, "illegal constructor", + v8::NewStringType::kNormal) + .ToLocalChecked()); + return; + } + if (info.Data().As()->Value() != FXJSOBJTYPE_DYNAMIC) { + isolate->ThrowException( + v8::String::NewFromUtf8(isolate, "not a dynamic object", + v8::NewStringType::kNormal) + .ToLocalChecked()); + return; + } + v8::Local holder = info.Holder(); + ASSERT(holder->InternalFieldCount() == 2); + holder->SetAlignedPointerInInternalField(0, nullptr); + holder->SetAlignedPointerInInternalField(1, nullptr); } v8::Isolate* GetIsolate() const { return m_pIsolate.Get(); } diff --git a/testing/resources/javascript/bug_695826.in b/testing/resources/javascript/bug_695826.in index 1b4e8e3faf..e17eaf11d9 100644 --- a/testing/resources/javascript/bug_695826.in +++ b/testing/resources/javascript/bug_695826.in @@ -35,8 +35,11 @@ endobj {{streamlen}} >> stream -var obj = new this.constructor; -obj.author = 3; +try { + var obj = new this.constructor; + obj.author = 3; +} catch (e) { +} app.alert('Done!'); endstream endobj diff --git a/testing/resources/javascript/constructor.in b/testing/resources/javascript/constructor.in new file mode 100644 index 0000000000..1211f89868 --- /dev/null +++ b/testing/resources/javascript/constructor.in @@ -0,0 +1,77 @@ +{{header}} +{{object 1 0}} << + /Type /Catalog + /Pages 2 0 R + /OpenAction 10 0 R +>> +endobj +{{object 2 0}} << + /Type /Pages + /Count 1 + /Kids [ + 3 0 R + ] +>> +endobj +% Page number 0. +{{object 3 0}} << + /Type /Page + /Parent 2 0 R + /MediaBox [0 0 612 792] +>> +% OpenAction action +{{object 10 0}} << + /Type /Action + /S /JavaScript + /JS 11 0 R +>> +endobj +% JS program to exexute +{{object 11 0}} << + {{streamlen}} +>> +stream +function testIllegalConstructor(name, allowed) { + const constructorString = name + ".constructor"; + let constructor; + try { + constructor = eval(constructorString); + } catch (e) { + app.alert("FAIL: No such " + constructorString); + return; + } + try { + constructor(); + app.alert("FAIL: " + constructorString + "(): returned"); + } catch (e) { + app.alert("PASS: " + constructorString + "(): " + e); + } + try { + new constructor; + app.alert("FAIL: new " + constructorString + ": returned"); + } catch (e) { + app.alert("PASS: new " + constructorString + ": " + e); + } +} +testIllegalConstructor("this"); +testIllegalConstructor("app"); +testIllegalConstructor("event"); +testIllegalConstructor("font"); +testIllegalConstructor("global"); +testIllegalConstructor("util"); +testIllegalConstructor("style"); +testIllegalConstructor("color"); +testIllegalConstructor("border"); +testIllegalConstructor("display"); +testIllegalConstructor("console"); +testIllegalConstructor("position"); +testIllegalConstructor("highlight"); +testIllegalConstructor("zoomtype"); +testIllegalConstructor("scaleHow"); +testIllegalConstructor("scaleWhen"); +endstream +endobj +{{xref}} +{{trailer}} +{{startxref}} +%%EOF diff --git a/testing/resources/javascript/constructor_expected.txt b/testing/resources/javascript/constructor_expected.txt new file mode 100644 index 0000000000..af0333735c --- /dev/null +++ b/testing/resources/javascript/constructor_expected.txt @@ -0,0 +1,32 @@ +Alert: PASS: this.constructor(): illegal constructor +Alert: PASS: new this.constructor: not a dynamic object +Alert: PASS: app.constructor(): illegal constructor +Alert: PASS: new app.constructor: not a dynamic object +Alert: PASS: event.constructor(): illegal constructor +Alert: PASS: new event.constructor: not a dynamic object +Alert: PASS: font.constructor(): illegal constructor +Alert: PASS: new font.constructor: not a dynamic object +Alert: PASS: global.constructor(): illegal constructor +Alert: PASS: new global.constructor: not a dynamic object +Alert: PASS: util.constructor(): illegal constructor +Alert: PASS: new util.constructor: not a dynamic object +Alert: PASS: style.constructor(): illegal constructor +Alert: PASS: new style.constructor: not a dynamic object +Alert: PASS: color.constructor(): illegal constructor +Alert: PASS: new color.constructor: not a dynamic object +Alert: PASS: border.constructor(): illegal constructor +Alert: PASS: new border.constructor: not a dynamic object +Alert: PASS: display.constructor(): illegal constructor +Alert: PASS: new display.constructor: not a dynamic object +Alert: PASS: console.constructor(): illegal constructor +Alert: PASS: new console.constructor: not a dynamic object +Alert: PASS: position.constructor(): illegal constructor +Alert: PASS: new position.constructor: not a dynamic object +Alert: PASS: highlight.constructor(): illegal constructor +Alert: PASS: new highlight.constructor: not a dynamic object +Alert: PASS: zoomtype.constructor(): illegal constructor +Alert: PASS: new zoomtype.constructor: not a dynamic object +Alert: PASS: scaleHow.constructor(): illegal constructor +Alert: PASS: new scaleHow.constructor: not a dynamic object +Alert: PASS: scaleWhen.constructor(): illegal constructor +Alert: PASS: new scaleWhen.constructor: not a dynamic object -- cgit v1.2.3