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 <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

4 files changed, 137 insertions, 11 deletions

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<v8::FunctionTemplate> fun = v8::FunctionTemplate::New(isolate);
     fun->InstanceTemplate()->SetInternalFieldCount(2);
-    fun->SetCallHandler([](const v8::FunctionCallbackInfo<v8::Value>& info) {
-      v8::Local<v8::Object> 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<v8::Signature> sig = v8::Signature::New(isolate, fun);
-    m_Signature.Reset(isolate, sig);
+  static void CallHandler(const v8::FunctionCallbackInfo<v8::Value>& 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<v8::Int32>()->Value() != FXJSOBJTYPE_DYNAMIC) {
+      isolate->ThrowException(
+          v8::String::NewFromUtf8(isolate, "not a dynamic object",
+                                  v8::NewStringType::kNormal)
+              .ToLocalChecked());
+      return;
+    }
+    v8::Local<v8::Object> 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