timers: make setImmediate() immune to tampering

Make setImmediate() immune to `process` global tampering by removing
the dependency on the `process._immediateCallback` property.

PR-URL: https://github.com/nodejs/node/pull/17736
Fixes: https://github.com/nodejs/node/issues/17681
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

4 files changed, 25 insertions, 17 deletions

diff --git a/src/env.cc b/src/env.cc
index 97d253ae1d..c3f42302ae 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -311,7 +311,7 @@ void Environment::CheckImmediate(uv_check_t* handle) {
 
   MakeCallback(env->isolate(),
                env->process_object(),
-               env->immediate_callback_string(),
+               env->immediate_callback_function(),
                0,
                nullptr,
                {0, 0}).ToLocalChecked();
diff --git a/src/env.h b/src/env.h
index 1ff75aa88b..2c77949bc5 100644
--- a/src/env.h
+++ b/src/env.h
@@ -158,7 +158,6 @@ class ModuleWrap;
   V(homedir_string, "homedir")                                                \
   V(hostmaster_string, "hostmaster")                                          \
   V(ignore_string, "ignore")                                                  \
-  V(immediate_callback_string, "_immediateCallback")                          \
   V(infoaccess_string, "infoAccess")                                          \
   V(inherit_string, "inherit")                                                \
   V(input_string, "input")                                                    \
@@ -289,6 +288,7 @@ class ModuleWrap;
   V(http2ping_constructor_template, v8::ObjectTemplate)                       \
   V(http2stream_constructor_template, v8::ObjectTemplate)                     \
   V(http2settings_constructor_template, v8::ObjectTemplate)                   \
+  V(immediate_callback_function, v8::Function)                                \
   V(inspector_console_api_object, v8::Object)                                 \
   V(module_load_list_array, v8::Array)                                        \
   V(pbkdf2_constructor_template, v8::ObjectTemplate)                          \
diff --git a/src/node.cc b/src/node.cc
index 0428d9b199..53c1001d4f 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -2939,12 +2939,6 @@ static void DebugEnd(const FunctionCallbackInfo<Value>& args);
 
 namespace {
 
-void ActivateImmediateCheck(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  env->ActivateImmediateCheck();
-}
-
-
 void StartProfilerIdleNotifier(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   env->StartProfilerIdleNotifier();
@@ -3168,12 +3162,6 @@ void SetupProcessObject(Environment* env,
                              FIXED_ONE_BYTE_STRING(env->isolate(), "ppid"),
                              GetParentProcessId).FromJust());
 
-  auto scheduled_immediate_count =
-      FIXED_ONE_BYTE_STRING(env->isolate(), "_scheduledImmediateCount");
-  CHECK(process->Set(env->context(),
-                     scheduled_immediate_count,
-                     env->scheduled_immediate_count().GetJSArray()).FromJust());
-
   auto should_abort_on_uncaught_toggle =
       FIXED_ONE_BYTE_STRING(env->isolate(), "_shouldAbortOnUncaughtToggle");
   CHECK(process->Set(env->context(),
@@ -3306,9 +3294,6 @@ void SetupProcessObject(Environment* env,
 
   // define various internal methods
   env->SetMethod(process,
-                 "_activateImmediateCheck",
-                 ActivateImmediateCheck);
-  env->SetMethod(process,
                  "_startProfilerIdleNotifier",
                  StartProfilerIdleNotifier);
   env->SetMethod(process,
diff --git a/src/timer_wrap.cc b/src/timer_wrap.cc
index 874c80d8d7..5c3f499d16 100644
--- a/src/timer_wrap.cc
+++ b/src/timer_wrap.cc
@@ -29,7 +29,9 @@
 namespace node {
 namespace {
 
+using v8::Array;
 using v8::Context;
+using v8::Function;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
 using v8::HandleScope;
@@ -67,11 +69,32 @@ class TimerWrap : public HandleWrap {
     env->SetProtoMethod(constructor, "stop", Stop);
 
     target->Set(timerString, constructor->GetFunction());
+
+    target->Set(env->context(),
+                FIXED_ONE_BYTE_STRING(env->isolate(), "setImmediateCallback"),
+                env->NewFunctionTemplate(SetImmediateCallback)
+                   ->GetFunction(env->context()).ToLocalChecked()).FromJust();
   }
 
   size_t self_size() const override { return sizeof(*this); }
 
  private:
+  static void SetImmediateCallback(const FunctionCallbackInfo<Value>& args) {
+    CHECK(args[0]->IsFunction());
+    auto env = Environment::GetCurrent(args);
+    env->set_immediate_callback_function(args[0].As<Function>());
+    auto activate_cb = [] (const FunctionCallbackInfo<Value>& args) {
+      Environment::GetCurrent(args)->ActivateImmediateCheck();
+    };
+    auto activate_function =
+        env->NewFunctionTemplate(activate_cb)->GetFunction(env->context())
+        .ToLocalChecked();
+    auto result = Array::New(env->isolate(), 2);
+    result->Set(0, activate_function);
+    result->Set(1, env->scheduled_immediate_count().GetJSArray());
+    args.GetReturnValue().Set(result);
+  }
+
   static void New(const FunctionCallbackInfo<Value>& args) {
     // This constructor should not be exposed to public javascript.
     // Therefore we assert that we are not trying to call this as a