timers: make setImmediate() immune to tampering

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

Fixes: nodejs#17681

lib/timers.js

@@ -22,7 +22,10 @@
 'use strict';
 
 const async_wrap = process.binding('async_wrap');
-const TimerWrap = process.binding('timer_wrap').Timer;
+const {
+  Timer: TimerWrap,
+  setImmediateCallback,
+} = process.binding('timer_wrap');
 const L = require('internal/linkedlist');
 const internalUtil = require('internal/util');
 const { createPromise, promiseResolve } = process.binding('util');
@@ -46,13 +49,8 @@ const { kInit, kDestroy, kAsyncIdCounter } = async_wrap.constants;
 // Symbols for storing async id state.
 const async_id_symbol = Symbol('asyncId');
 const trigger_async_id_symbol = Symbol('triggerAsyncId');
-
-/* This is an Uint32Array for easier sharing with C++ land. */
-const scheduledImmediateCount = process._scheduledImmediateCount;
-delete process._scheduledImmediateCount;
-/* Kick off setImmediate processing */
-const activateImmediateCheck = process._activateImmediateCheck;
-delete process._activateImmediateCheck;
+const [activateImmediateCheck, scheduledImmediateCountArray] =
+  setImmediateCallback(processImmediate);
 
 // Timeout values > TIMEOUT_MAX are set to 1.
 const TIMEOUT_MAX = 2 ** 31 - 1;
@@ -722,8 +720,6 @@ function processImmediate() {
   }
 }
 
-process._immediateCallback = processImmediate;
-
 // An optimization so that the try/finally only de-optimizes (since at least v8
 // 4.7) what is in this smaller function.
 function tryOnImmediate(immediate, oldTail) {
@@ -740,7 +736,7 @@ function tryOnImmediate(immediate, oldTail) {
 
     if (!immediate._destroyed) {
       immediate._destroyed = true;
-      scheduledImmediateCount[0]--;
+      scheduledImmediateCountArray[0]--;
 
       if (async_hook_fields[kDestroy] > 0) {
         emitDestroy(immediate[async_id_symbol]);
@@ -794,9 +790,9 @@ function Immediate(callback, args) {
              this);
   }
 
-  if (scheduledImmediateCount[0] === 0)
+  if (scheduledImmediateCountArray[0] === 0)
     activateImmediateCheck();
-  scheduledImmediateCount[0]++;
+  scheduledImmediateCountArray[0]++;
 
   immediateQueue.append(this);
 }
@@ -842,7 +838,7 @@ exports.clearImmediate = function(immediate) {
   if (!immediate) return;
 
   if (!immediate._destroyed) {
-    scheduledImmediateCount[0]--;
+    scheduledImmediateCountArray[0]--;
     immediate._destroyed = true;
 
     if (async_hook_fields[kDestroy] > 0) {

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();

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")                                                    \
@@ -288,6 +287,7 @@ class ModuleWrap;
   V(host_import_module_dynamically_callback, v8::Function)                    \
   V(http2ping_constructor_template, v8::ObjectTemplate)                       \
   V(http2stream_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)                          \

src/node.cc

@@ -2932,12 +2932,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();
@@ -3161,12 +3155,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(),
@@ -3298,9 +3286,6 @@ void SetupProcessObject(Environment* env,
                              env->as_external()).FromJust());
 
   // define various internal methods
-  env->SetMethod(process,
-                 "_activateImmediateCheck",
-                 ActivateImmediateCheck);
   env->SetMethod(process,
                  "_startProfilerIdleNotifier",
                  StartProfilerIdleNotifier);

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

test/common/index.js

@@ -21,6 +21,7 @@
 
 /* eslint-disable required-modules, crypto-check */
 'use strict';
+const process = global.process;  // Some tests tamper with the process global.
 const path = require('path');
 const fs = require('fs');
 const assert = require('assert');

test/parallel/test-timer-immediate.js

@@ -0,0 +1,5 @@
+'use strict';
+const common = require('../common');
+common.globalCheck = false;
+process = {};  // Boom!
+setImmediate(common.mustCall());