module: apply null bytes check to module read path

PR-URL: #8277

Author: rvagg

lib/fs.js

@@ -130,6 +130,9 @@ function assertEncoding(encoding) {
   }
 }
 
+// This is duplicated in module.js and needs to be moved to internal/fs.js
+// once it is OK again to include internal/ resources in fs.js.
+// See: https://github.com/nodejs/node/pull/6413
 function nullCheck(path, callback) {
   if (('' + path).indexOf('\u0000') !== -1) {
     var er = new Error('Path must be a string without null bytes');

lib/module.js

@@ -34,6 +34,22 @@ function stat(filename) {
 stat.cache = null;
 
 
+// This is duplicated from fs.js and needs to be moved to internal/fs.js
+// once it is OK again to include internal/ resources in fs.js.
+// See: https://github.com/nodejs/node/pull/6413
+function nullCheck(path, callback) {
+  if (('' + path).indexOf('\u0000') !== -1) {
+    var er = new Error('Path must be a string without null bytes');
+    er.code = 'ENOENT';
+    if (typeof callback !== 'function')
+      throw er;
+    process.nextTick(callback, er);
+    return false;
+  }
+  return true;
+}
+
+
 function Module(id, parent) {
   this.id = id;
   this.exports = {};
@@ -135,6 +151,8 @@ function tryExtensions(p, exts, isMain) {
 
 var warned = false;
 Module._findPath = function(request, paths, isMain) {
+  nullCheck(request);
+
   if (path.isAbsolute(request)) {
     paths = [''];
   } else if (!paths || paths.length === 0) {

test/parallel/test-fs-null-bytes.js

@@ -2,56 +2,74 @@
 var common = require('../common');
 var assert = require('assert');
 var fs = require('fs');
+var expectedError = /Path must be a string without null bytes/;
 
 function check(async, sync) {
-  var expected = /Path must be a string without null bytes/;
   var argsSync = Array.prototype.slice.call(arguments, 2);
   var argsAsync = argsSync.concat(function(er) {
-    assert(er && er.message.match(expected));
+    assert(er && er.message.match(expectedError));
     assert.equal(er.code, 'ENOENT');
   });
 
   if (sync)
     assert.throws(function() {
-      console.error(sync.name, argsSync);
-      sync.apply(null, argsSync);
-    }, expected);
+      console.error(`fs.${sync}()`, argsSync);
+      fs[sync].apply(null, argsSync);
+    }, expectedError);
 
-  if (async)
-    async.apply(null, argsAsync);
+  if (async) {
+    console.error(`fs.${async}()`, argsAsync);
+    fs[async].apply(null, argsAsync);
+  }
 }
 
-check(fs.access, fs.accessSync, 'foo\u0000bar');
-check(fs.access, fs.accessSync, 'foo\u0000bar', fs.F_OK);
-check(fs.appendFile, fs.appendFileSync, 'foo\u0000bar');
-check(fs.chmod, fs.chmodSync, 'foo\u0000bar', '0644');
-check(fs.chown, fs.chownSync, 'foo\u0000bar', 12, 34);
-check(fs.link, fs.linkSync, 'foo\u0000bar', 'foobar');
-check(fs.link, fs.linkSync, 'foobar', 'foo\u0000bar');
-check(fs.lstat, fs.lstatSync, 'foo\u0000bar');
-check(fs.mkdir, fs.mkdirSync, 'foo\u0000bar', '0755');
-check(fs.open, fs.openSync, 'foo\u0000bar', 'r');
-check(fs.readFile, fs.readFileSync, 'foo\u0000bar');
-check(fs.readdir, fs.readdirSync, 'foo\u0000bar');
-check(fs.readlink, fs.readlinkSync, 'foo\u0000bar');
-check(fs.realpath, fs.realpathSync, 'foo\u0000bar');
-check(fs.rename, fs.renameSync, 'foo\u0000bar', 'foobar');
-check(fs.rename, fs.renameSync, 'foobar', 'foo\u0000bar');
-check(fs.rmdir, fs.rmdirSync, 'foo\u0000bar');
-check(fs.stat, fs.statSync, 'foo\u0000bar');
-check(fs.symlink, fs.symlinkSync, 'foo\u0000bar', 'foobar');
-check(fs.symlink, fs.symlinkSync, 'foobar', 'foo\u0000bar');
-check(fs.truncate, fs.truncateSync, 'foo\u0000bar');
-check(fs.unlink, fs.unlinkSync, 'foo\u0000bar');
-check(null, fs.unwatchFile, 'foo\u0000bar', common.fail);
-check(fs.utimes, fs.utimesSync, 'foo\u0000bar', 0, 0);
-check(null, fs.watch, 'foo\u0000bar', common.fail);
-check(null, fs.watchFile, 'foo\u0000bar', common.fail);
-check(fs.writeFile, fs.writeFileSync, 'foo\u0000bar');
+check('access', 'accessSync', 'foo\u0000bar');
+check('access', 'accessSync', 'foo\u0000bar', 'F_OK');
+check('appendFile', 'appendFileSync', 'foo\u0000bar');
+check('chmod', 'chmodSync', 'foo\u0000bar', '0644');
+check('chown', 'chownSync', 'foo\u0000bar', 12, 34);
+check('link', 'linkSync', 'foo\u0000bar', 'foobar');
+check('link', 'linkSync', 'foobar', 'foo\u0000bar');
+check('lstat', 'lstatSync', 'foo\u0000bar');
+check('mkdir', 'mkdirSync', 'foo\u0000bar', '0755');
+check('open', 'openSync', 'foo\u0000bar', 'r');
+check('readFile', 'readFileSync', 'foo\u0000bar');
+check('readdir', 'readdirSync', 'foo\u0000bar');
+check('readlink', 'readlinkSync', 'foo\u0000bar');
+check('realpath', 'realpathSync', 'foo\u0000bar');
+check('rename', 'renameSync', 'foo\u0000bar', 'foobar');
+check('rename', 'renameSync', 'foobar', 'foo\u0000bar');
+check('rmdir', 'rmdirSync', 'foo\u0000bar');
+check('stat', 'statSync', 'foo\u0000bar');
+check('symlink', 'symlinkSync', 'foo\u0000bar', 'foobar');
+check('symlink', 'symlinkSync', 'foobar', 'foo\u0000bar');
+check('truncate', 'truncateSync', 'foo\u0000bar');
+check('unlink', 'unlinkSync', 'foo\u0000bar');
+check(null, 'unwatchFile', 'foo\u0000bar', common.fail);
+check('utimes', 'utimesSync', 'foo\u0000bar', 0, 0);
+check(null, 'watch', 'foo\u0000bar', common.fail);
+check(null, 'watchFile', 'foo\u0000bar', common.fail);
+check('writeFile', 'writeFileSync', 'foo\u0000bar');
 
 // an 'error' for exists means that it doesn't exist.
 // one of many reasons why this file is the absolute worst.
 fs.exists('foo\u0000bar', function(exists) {
   assert(!exists);
 });
 assert(!fs.existsSync('foo\u0000bar'));
+
+function checkRequire(arg) {
+  assert.throws(function() {
+    console.error(`require(${JSON.stringify(arg)})`);
+    require(arg);
+  }, expectedError);
+}
+
+checkRequire('\u0000');
+checkRequire('foo\u0000bar');
+checkRequire('foo\u0000');
+checkRequire('foo/\u0000');
+checkRequire('foo/\u0000.js');
+checkRequire('\u0000/foo');
+checkRequire('./foo/\u0000');
+checkRequire('./\u0000/foo');