lib: make primordials Promise methods safe

`catch` and `finally` methods on %Promise.prototype% looks up the `then`
property of the instance, making it at risk of prototype pollution.

PR-URL: https://github.com/nodejs/node/pull/38650
Refs: https://tc39.es/ecma262/#sec-promise.prototype.catch
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

4 files changed, 43 insertions, 9 deletions

diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
index 0253a29c7d5..b30af8fdf55 100644
--- a/lib/internal/fs/promises.js
+++ b/lib/internal/fs/promises.js
@@ -7,11 +7,11 @@ const {
   MathMin,
   NumberIsSafeInteger,
   Promise,
-  PromisePrototypeFinally,
   PromisePrototypeThen,
   PromiseResolve,
   PromiseReject,
   SafeArrayIterator,
+  SafePromisePrototypeFinally,
   Symbol,
   Uint8Array,
 } = primordials;
@@ -188,12 +188,12 @@ class FileHandle extends EventEmitterMixin(JSTransferable) {
     this[kRefs]--;
     if (this[kRefs] === 0) {
       this[kFd] = -1;
-      this[kClosePromise] = PromisePrototypeFinally(
+      this[kClosePromise] = SafePromisePrototypeFinally(
         this[kHandle].close(),
         () => { this[kClosePromise] = undefined; }
       );
     } else {
-      this[kClosePromise] = PromisePrototypeFinally(
+      this[kClosePromise] = SafePromisePrototypeFinally(
         new Promise((resolve, reject) => {
           this[kCloseResolve] = resolve;
           this[kCloseReject] = reject;
diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js
index bc6f90ce7d0..b8238a1dd2c 100644
--- a/lib/internal/modules/run_main.js
+++ b/lib/internal/modules/run_main.js
@@ -1,7 +1,6 @@
 'use strict';
 
 const {
-  PromisePrototypeFinally,
   StringPrototypeEndsWith,
 } = primordials;
 const CJSLoader = require('internal/modules/cjs/loader');
@@ -51,7 +50,7 @@ function runMainESM(mainPath) {
   }));
 }
 
-function handleMainPromise(promise) {
+async function handleMainPromise(promise) {
   // Handle a Promise from running code that potentially does Top-Level Await.
   // In that case, it makes sense to set the exit code to a specific non-zero
   // value if the main code never finishes running.
@@ -60,7 +59,11 @@ function handleMainPromise(promise) {
       process.exitCode = 13;
   }
   process.on('exit', handler);
-  return PromisePrototypeFinally(promise, () => process.off('exit', handler));
+  try {
+    return await promise;
+  } finally {
+    process.off('exit', handler);
+  }
 }
 
 // For backwards compatibility, we have to run a bunch of
diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js
index 78f778b6570..42250ffb422 100644
--- a/lib/internal/per_context/primordials.js
+++ b/lib/internal/per_context/primordials.js
@@ -253,6 +253,8 @@ const {
   Map,
   ObjectFreeze,
   ObjectSetPrototypeOf,
+  Promise,
+  PromisePrototypeThen,
   Set,
   SymbolIterator,
   WeakMap,
@@ -384,5 +386,34 @@ primordials.SafeWeakRef = makeSafe(
   }
 );
 
+const SafePromise = makeSafe(
+  Promise,
+  class SafePromise extends Promise {
+    // eslint-disable-next-line no-useless-constructor
+    constructor(executor) { super(executor); }
+  }
+);
+
+primordials.PromisePrototypeCatch = (thisPromise, onRejected) =>
+  PromisePrototypeThen(thisPromise, undefined, onRejected);
+
+/**
+ * Attaches a callback that is invoked when the Promise is settled (fulfilled or
+ * rejected). The resolved value cannot be modified from the callback.
+ * Prefer using async functions when possible.
+ * @param {Promise<any>} thisPromise
+ * @param {() => void) | undefined | null} onFinally The callback to execute
+ *        when the Promise is settled (fulfilled or rejected).
+ * @returns A Promise for the completion of the callback.
+ */
+primordials.SafePromisePrototypeFinally = (thisPromise, onFinally) =>
+  // Wrapping on a new Promise is necessary to not expose the SafePromise
+  // prototype to user-land.
+  new Promise((a, b) =>
+    new SafePromise((a, b) => PromisePrototypeThen(thisPromise, a, b))
+      .finally(onFinally)
+      .then(a, b)
+  );
+
 ObjectSetPrototypeOf(primordials, null);
 ObjectFreeze(primordials);
diff --git a/lib/timers/promises.js b/lib/timers/promises.js
index 1f245580f86..162f465da29 100644
--- a/lib/timers/promises.js
+++ b/lib/timers/promises.js
@@ -3,8 +3,8 @@
 const {
   FunctionPrototypeBind,
   Promise,
-  PromisePrototypeFinally,
   PromiseReject,
+  SafePromisePrototypeFinally,
 } = primordials;
 
 const {
@@ -71,7 +71,7 @@ function setTimeout(after, value, options = {}) {
     }
   });
   return oncancel !== undefined ?
-    PromisePrototypeFinally(
+    SafePromisePrototypeFinally(
       ret,
       () => signal.removeEventListener('abort', oncancel)) : ret;
 }
@@ -115,7 +115,7 @@ function setImmediate(value, options = {}) {
     }
   });
   return oncancel !== undefined ?
-    PromisePrototypeFinally(
+    SafePromisePrototypeFinally(
       ret,
       () => signal.removeEventListener('abort', oncancel)) : ret;
 }