Reject non-string prefix, postfix, template

The relative-value guard in `_assertPath` calls `.includes('..')` directly on
the user-supplied value. When the value is an Array the call checks element
equality (so `['../escape'].includes('..')` is false), and when the value is
an arbitrary object a duck-typed `includes` returning false defeats the check
entirely. In both cases the value is subsequently coerced to a string by
`Array.prototype.join` inside `_generateTmpName` and by `path.join`, so a
non-string carrying `../` still produces a path that escapes `tmpdir`.

Tighten `_assertPath` to require `typeof value === 'string'` before the
substring check, and apply the same type check to `template` ahead of the
existing `XXXXXX` regex match (otherwise `match` throws on a non-string with
an unrelated error). The error includes the option name so consumers can see
which option was wrong.

Adds a `test/GHSA-7c78-jf6q-g5cm-test.js` that exercises array, duck-typed
object, and primitive (number) inputs across `fileSync`, `dirSync`, and
`tmpNameSync`, and asserts that valid string inputs are still accepted.

Signed-off-by: tonghuaroot <tonghuaroot@gmail.com>

Author: tonghuaroot

lib/tmp.js

@@ -526,16 +526,26 @@ function _generateTmpName(opts) {
 }
 
 /**
- * Check the prefix and postfix options
+ * Check the prefix, postfix, and template options.
+ *
+ * Rejects non-string inputs so that a non-string `.includes('..')` cannot evade
+ * the substring check (e.g. an Array whose `.includes('..')` is element-wise,
+ * or a duck-typed object with a custom `.includes`), and so that the value is
+ * not later coerced to a string with traversal sequences via `Array.prototype.join`
+ * or `path.join`.
  *
  * @private
  */
-function _assertPath(path) {
-  if (path.includes("..")) {
+function _assertPath(option, value) {
+  if (typeof value !== 'string') {
+    throw new Error(`${option} option must be a string, got "${typeof value}".`);
+  }
+
+  if (value.includes("..")) {
     throw new Error("Relative value not allowed");
   }
 
-  return path;
+  return value;
 }
 
 /**
@@ -558,8 +568,13 @@ function _assertOptionsBase(options) {
   }
 
   /* istanbul ignore else */
-  if (!_isUndefined(options.template) && !options.template.match(TEMPLATE_PATTERN)) {
-    throw new Error(`Invalid template, found "${options.template}".`);
+  if (!_isUndefined(options.template)) {
+    if (typeof options.template !== 'string') {
+      throw new Error(`template option must be a string, got "${typeof options.template}".`);
+    }
+    if (!options.template.match(TEMPLATE_PATTERN)) {
+      throw new Error(`Invalid template, found "${options.template}".`);
+    }
   }
 
   /* istanbul ignore else */
@@ -575,9 +590,9 @@ function _assertOptionsBase(options) {
   options.unsafeCleanup = !!options.unsafeCleanup;
 
   // for completeness' sake only, also keep (multiple) blanks if the user, purportedly sane, requests us to
-  options.prefix = _isUndefined(options.prefix) ? '' : _assertPath(options.prefix);
-  options.postfix = _isUndefined(options.postfix) ? '' : _assertPath(options.postfix);
-  options.template = _isUndefined(options.template) ? undefined : _assertPath(options.template);
+  options.prefix = _isUndefined(options.prefix) ? '' : _assertPath('prefix', options.prefix);
+  options.postfix = _isUndefined(options.postfix) ? '' : _assertPath('postfix', options.postfix);
+  options.template = _isUndefined(options.template) ? undefined : _assertPath('template', options.template);
 }
 
 /**

test/GHSA-7c78-jf6q-g5cm-test.js

@@ -0,0 +1,81 @@
+const assert = require('assert');
+const tmp = require('../lib/tmp');
+
+describe('GHSA-7c78-jf6q-g5cm', function () {
+  describe('#fileSync with non-string `prefix`', function () {
+    it('should reject an array prefix even when its element is "../foo"', function (done) {
+      assert.throws(function () {
+        tmp.fileSync({ prefix: ['../foo'] });
+      }, new RegExp('^Error: prefix option must be a string'));
+
+      done();
+    });
+
+    it('should reject a duck-typed object whose includes() returns false', function (done) {
+      assert.throws(function () {
+        tmp.fileSync({
+          prefix: { toString: function () { return '../foo'; }, includes: function () { return false; } }
+        });
+      }, new RegExp('^Error: prefix option must be a string'));
+
+      done();
+    });
+
+    it('should reject a number prefix', function (done) {
+      assert.throws(function () {
+        tmp.fileSync({ prefix: 42 });
+      }, new RegExp('^Error: prefix option must be a string'));
+
+      done();
+    });
+  });
+
+  describe('#fileSync with non-string `postfix`', function () {
+    it('should reject an array postfix', function (done) {
+      assert.throws(function () {
+        tmp.fileSync({ postfix: ['/../foo'] });
+      }, new RegExp('^Error: postfix option must be a string'));
+
+      done();
+    });
+  });
+
+  describe('#fileSync with non-string `template`', function () {
+    it('should reject an array template', function (done) {
+      assert.throws(function () {
+        tmp.fileSync({ template: ['XXXXXX/../foo'] });
+      }, new RegExp('^Error: template option must be a string'));
+
+      done();
+    });
+  });
+
+  describe('#dirSync with non-string `prefix`', function () {
+    it('should reject an array prefix', function (done) {
+      assert.throws(function () {
+        tmp.dirSync({ prefix: ['../escape'] });
+      }, new RegExp('^Error: prefix option must be a string'));
+
+      done();
+    });
+  });
+
+  describe('#tmpNameSync with non-string `prefix`', function () {
+    it('should reject an array prefix', function (done) {
+      assert.throws(function () {
+        tmp.tmpNameSync({ prefix: ['../escape'] });
+      }, new RegExp('^Error: prefix option must be a string'));
+
+      done();
+    });
+  });
+
+  describe('valid string prefixes still work', function () {
+    it('should accept a normal string prefix', function (done) {
+      const r = tmp.fileSync({ prefix: 'safe-prefix' });
+      assert.ok(r.name.indexOf('safe-prefix') !== -1);
+      r.removeCallback();
+      done();
+    });
+  });
+});