fs: ensure nullCheck() callback is a function

Currently, nullCheck() will attempt to invoke any truthy value
as a function if the path argument contains a null character.
This commit validates that the callback is actually a function
before trying to invoke it. fs.access() was vulnerable to this
bug, as nullCheck() was called prior to type checking its
callback.

PR-URL: https://github.com/iojs/io.js/pull/887
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

diff --git a/lib/fs.js b/lib/fs.js
index 18332c7..7a89362 100644 (file)
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -84,7 +84,7 @@ 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 (!callback)
+    if (typeof callback !== 'function')
       throw er;
     process.nextTick(function() {
       callback(er);
@@ -169,9 +169,6 @@ fs.Stats.prototype.isSocket = function() {
 });
 
 fs.access = function(path, mode, callback) {
-  if (!nullCheck(path, callback))
-    return;
-
   if (typeof mode === 'function') {
     callback = mode;
     mode = fs.F_OK;
@@ -179,6 +176,9 @@ fs.access = function(path, mode, callback) {
     throw new TypeError('callback must be a function');
   }
 
+  if (!nullCheck(path, callback))
+    return;
+
   mode = mode | 0;
   var req = new FSReqWrap();
   req.oncomplete = makeCallback(callback);

diff --git a/test/parallel/test-fs-access.js b/test/parallel/test-fs-access.js
index 3846b7b..cf8dd94 100644 (file)
--- a/test/parallel/test-fs-access.js
+++ b/test/parallel/test-fs-access.js
@@ -57,6 +57,10 @@ assert.throws(function() {
   fs.access(__filename, fs.F_OK);
 }, /callback must be a function/);
 
+assert.throws(function() {
+  fs.access(__filename, fs.F_OK, {});
+}, /callback must be a function/);
+
 assert.doesNotThrow(function() {
   fs.accessSync(__filename);
 });

diff --git a/test/parallel/test-fs-null-bytes.js b/test/parallel/test-fs-null-bytes.js
index 8499c03..4d1dcb3 100644 (file)
--- a/test/parallel/test-fs-null-bytes.js
+++ b/test/parallel/test-fs-null-bytes.js
@@ -20,6 +20,8 @@ function check(async, sync) {
     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);
@@ -52,4 +54,3 @@ fs.exists('foo\u0000bar', function(exists) {
   assert(!exists);
 });
 assert(!fs.existsSync('foo\u0000bar'));
-