Merge commit from fork

Reject non-string prefix, postfix, template

Author: raszi

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