From 2ccd4dc564b93a46246e73604c4a6ef0ddb89f99 Mon Sep 17 00:00:00 2001 From: Devlin Cronin Date: Mon, 2 Nov 2020 14:01:19 -0800 Subject: [PATCH] Introduce Function::FunctionProtoToString() Add a new function on the public API to allow serializing a function to a string using the built-in toString() implementation, allowing serialization without worrying about untrusted author script overriding the toString() implementation. This is similar in nature to Object::ObjectProtoToString() (but that only returns "[object Function]" for any passed function). Add tests for the same. Bug: chromium:1144841 Change-Id: Ie4c29b870034c0817c23bf91f9424f956098823d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2514768 Reviewed-by: Toon Verwaest Commit-Queue: Devlin Cr-Commit-Position: refs/heads/master@{#70976} --- include/v8.h | 9 +++++++++ src/api/api.cc | 12 ++++++++++++ src/init/bootstrapper.cc | 6 ++++-- src/logging/counters.h | 1 + src/objects/contexts.h | 1 + test/cctest/test-api.cc | 27 +++++++++++++++++++++++++++ 6 files changed, 54 insertions(+), 2 deletions(-) diff --git a/include/v8.h b/include/v8.h index 9eb49043d3..835178204a 100644 --- a/include/v8.h +++ b/include/v8.h @@ -4621,6 +4621,15 @@ class V8_EXPORT Function : public Object { */ Local GetBoundFunction() const; + /** + * Calls builtin Function.prototype.toString on this function. + * This is different from Value::ToString() that may call a user-defined + * toString() function, and different than Object::ObjectProtoToString() which + * always serializes "[object Function]". + */ + V8_WARN_UNUSED_RESULT MaybeLocal FunctionProtoToString( + Local context); + ScriptOrigin GetScriptOrigin() const; V8_INLINE static Function* Cast(Value* obj); static const int kLineOffsetNotFound; diff --git a/src/api/api.cc b/src/api/api.cc index 5cc0df9f7a..9cfedf2f21 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -5126,6 +5126,18 @@ Local Function::GetBoundFunction() const { return v8::Undefined(reinterpret_cast(self->GetIsolate())); } +MaybeLocal v8::Function::FunctionProtoToString(Local context) { + PREPARE_FOR_EXECUTION(context, Function, FunctionProtoToString, String); + auto self = Utils::OpenHandle(this); + Local result; + has_pending_exception = !ToLocal( + i::Execution::CallBuiltin(isolate, isolate->function_to_string(), self, 0, + nullptr), + &result); + RETURN_ON_FAILED_EXECUTION(String); + RETURN_ESCAPED(Local::Cast(result)); +} + int Name::GetIdentityHash() { auto self = Utils::OpenHandle(this); return static_cast(self->Hash()); diff --git a/src/init/bootstrapper.cc b/src/init/bootstrapper.cc index fa82b0f0b2..b1a3361919 100644 --- a/src/init/bootstrapper.cc +++ b/src/init/bootstrapper.cc @@ -1571,8 +1571,10 @@ void Genesis::InitializeGlobal(Handle global_object, Builtins::kFastFunctionPrototypeBind, 1, false); SimpleInstallFunction(isolate_, prototype, "call", Builtins::kFunctionPrototypeCall, 1, false); - SimpleInstallFunction(isolate_, prototype, "toString", - Builtins::kFunctionPrototypeToString, 0, false); + Handle function_to_string = + SimpleInstallFunction(isolate_, prototype, "toString", + Builtins::kFunctionPrototypeToString, 0, false); + native_context()->set_function_to_string(*function_to_string); // Install the @@hasInstance function. Handle has_instance = InstallFunctionAtSymbol( diff --git a/src/logging/counters.h b/src/logging/counters.h index cd558a7582..cb879e3c23 100644 --- a/src/logging/counters.h +++ b/src/logging/counters.h @@ -739,6 +739,7 @@ class RuntimeCallTimer final { V(Float64Array_New) \ V(Function_Call) \ V(Function_New) \ + V(Function_FunctionProtoToString) \ V(Function_NewInstance) \ V(FunctionTemplate_GetFunction) \ V(FunctionTemplate_New) \ diff --git a/src/objects/contexts.h b/src/objects/contexts.h index e0238f527b..f62e41c9a8 100644 --- a/src/objects/contexts.h +++ b/src/objects/contexts.h @@ -311,6 +311,7 @@ enum ContextLookupFlags { V(FINALIZATION_REGISTRY_CLEANUP_SOME, JSFunction, \ finalization_registry_cleanup_some) \ V(FUNCTION_HAS_INSTANCE_INDEX, JSFunction, function_has_instance) \ + V(FUNCTION_TO_STRING_INDEX, JSFunction, function_to_string) \ V(OBJECT_TO_STRING, JSFunction, object_to_string) \ V(OBJECT_VALUE_OF_FUNCTION_INDEX, JSFunction, object_value_of_function) \ V(PROMISE_ALL_INDEX, JSFunction, promise_all) \ diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 8ad10ea112..bf4b9ff0e0 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -17566,6 +17566,33 @@ THREADED_TEST(FunctionGetBoundFunction) { original_function->GetScriptColumnNumber()); } +THREADED_TEST(FunctionProtoToString) { + LocalContext context; + v8::Isolate* isolate = CcTest::isolate(); + v8::HandleScope scope(isolate); + + // Replace Function.prototype.toString. + CompileRun(R"( + Function.prototype.toString = function() { + return 'customized toString'; + })"); + + constexpr char kTestFunction[] = "function testFunction() { return 7; }"; + std::string wrapped_function("("); + wrapped_function.append(kTestFunction).append(")"); + Local function = + CompileRun(wrapped_function.c_str()).As(); + + Local value = function->ToString(context.local()).ToLocalChecked(); + CHECK(value->IsString()); + CHECK( + value->Equals(context.local(), v8_str("customized toString")).FromJust()); + + // FunctionProtoToString() should not call the replaced toString function. + value = function->FunctionProtoToString(context.local()).ToLocalChecked(); + CHECK(value->IsString()); + CHECK(value->Equals(context.local(), v8_str(kTestFunction)).FromJust()); +} static void GetterWhichReturns42( Local name,