From 33fa7405778444ca66470ab0729e6fa9fe43d2a6 Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 7 Dec 2012 16:50:12 -0800 Subject: [PATCH] fs: Raise error when null bytes detected in paths Reworking of @bnoordhuis's more aggressive approach. --- lib/fs.js | 97 ++++++++++++++++++++++++++----- test/simple/test-fs-null-bytes.js | 75 ++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 14 deletions(-) create mode 100644 test/simple/test-fs-null-bytes.js diff --git a/lib/fs.js b/lib/fs.js index dbca4775490..8746740b22e 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -98,6 +98,18 @@ function assertEncoding(encoding) { } } +function nullCheck(path, callback) { + if (('' + path).indexOf('\u0000') !== -1) { + var er = new Error('Path must be a string without null bytes.'); + if (!callback) + throw er; + process.nextTick(function() { + callback(er); + }); + return false; + } + return true; +} fs.Stats = binding.Stats; @@ -134,13 +146,16 @@ fs.Stats.prototype.isSocket = function() { }; fs.exists = function(path, callback) { - binding.stat(pathModule._makeLong(path), function(err, stats) { + if (!nullCheck(path, cb)) return; + binding.stat(pathModule._makeLong(path), cb); + function cb(err, stats) { if (callback) callback(err ? false : true); - }); + } }; fs.existsSync = function(path) { try { + nullCheck(path); binding.stat(pathModule._makeLong(path)); return true; } catch (e) { @@ -362,6 +377,7 @@ fs.open = function(path, flags, mode, callback) { callback = makeCallback(arguments[arguments.length - 1]); mode = modeNum(mode, 438 /*=0666*/); + if (!nullCheck(path, callback)) return; binding.open(pathModule._makeLong(path), stringToFlags(flags), mode, @@ -370,6 +386,7 @@ fs.open = function(path, flags, mode, callback) { fs.openSync = function(path, flags, mode) { mode = modeNum(mode, 438 /*=0666*/); + nullCheck(path); return binding.open(pathModule._makeLong(path), stringToFlags(flags), mode); }; @@ -475,12 +492,17 @@ fs.writeSync = function(fd, buffer, offset, length, position) { }; fs.rename = function(oldPath, newPath, callback) { + callback = makeCallback(callback); + if (!nullCheck(oldPath, callback)) return; + if (!nullCheck(newPath, callback)) return; binding.rename(pathModule._makeLong(oldPath), pathModule._makeLong(newPath), - makeCallback(callback)); + callback); }; fs.renameSync = function(oldPath, newPath) { + nullCheck(oldPath); + nullCheck(newPath); return binding.rename(pathModule._makeLong(oldPath), pathModule._makeLong(newPath)); }; @@ -543,10 +565,13 @@ fs.ftruncateSync = function(fd, len) { }; fs.rmdir = function(path, callback) { - binding.rmdir(pathModule._makeLong(path), makeCallback(callback)); + callback = makeCallback(callback); + if (!nullCheck(path, callback)) return; + binding.rmdir(pathModule._makeLong(path), callback); }; fs.rmdirSync = function(path) { + nullCheck(path); return binding.rmdir(pathModule._makeLong(path)); }; @@ -568,12 +593,15 @@ fs.fsyncSync = function(fd) { fs.mkdir = function(path, mode, callback) { if (typeof mode === 'function') callback = mode; + callback = makeCallback(callback); + if (!nullCheck(path, callback)) return; binding.mkdir(pathModule._makeLong(path), modeNum(mode, 511 /*=0777*/), - makeCallback(callback)); + callback); }; fs.mkdirSync = function(path, mode) { + nullCheck(path); return binding.mkdir(pathModule._makeLong(path), modeNum(mode, 511 /*=0777*/)); }; @@ -587,10 +615,13 @@ fs.sendfileSync = function(outFd, inFd, inOffset, length) { }; fs.readdir = function(path, callback) { - binding.readdir(pathModule._makeLong(path), makeCallback(callback)); + callback = makeCallback(callback); + if (!nullCheck(path, callback)) return; + binding.readdir(pathModule._makeLong(path), callback); }; fs.readdirSync = function(path) { + nullCheck(path); return binding.readdir(pathModule._makeLong(path)); }; @@ -599,11 +630,15 @@ fs.fstat = function(fd, callback) { }; fs.lstat = function(path, callback) { - binding.lstat(pathModule._makeLong(path), makeCallback(callback)); + callback = makeCallback(callback); + if (!nullCheck(path, callback)) return; + binding.lstat(pathModule._makeLong(path), callback); }; fs.stat = function(path, callback) { - binding.stat(pathModule._makeLong(path), makeCallback(callback)); + callback = makeCallback(callback); + if (!nullCheck(path, callback)) return; + binding.stat(pathModule._makeLong(path), callback); }; fs.fstatSync = function(fd) { @@ -611,18 +646,23 @@ fs.fstatSync = function(fd) { }; fs.lstatSync = function(path) { + nullCheck(path); return binding.lstat(pathModule._makeLong(path)); }; fs.statSync = function(path) { + nullCheck(path); return binding.stat(pathModule._makeLong(path)); }; fs.readlink = function(path, callback) { - binding.readlink(pathModule._makeLong(path), makeCallback(callback)); + callback = makeCallback(callback); + if (!nullCheck(path, callback)) return; + binding.readlink(pathModule._makeLong(path), callback); }; fs.readlinkSync = function(path) { + nullCheck(path); return binding.readlink(pathModule._makeLong(path)); }; @@ -643,6 +683,9 @@ fs.symlink = function(destination, path, type_, callback) { var type = (typeof type_ === 'string' ? type_ : null); var callback = makeCallback(arguments[arguments.length - 1]); + if (!nullCheck(destination, callback)) return; + if (!nullCheck(path, callback)) return; + binding.symlink(preprocessSymlinkDestination(destination, type), pathModule._makeLong(path), type, @@ -652,27 +695,39 @@ fs.symlink = function(destination, path, type_, callback) { fs.symlinkSync = function(destination, path, type) { type = (typeof type === 'string' ? type : null); + nullCheck(destination); + nullCheck(path); + return binding.symlink(preprocessSymlinkDestination(destination, type), pathModule._makeLong(path), type); }; fs.link = function(srcpath, dstpath, callback) { + callback = makeCallback(callback); + if (!nullCheck(srcpath, callback)) return; + if (!nullCheck(dstpath, callback)) return; + binding.link(pathModule._makeLong(srcpath), pathModule._makeLong(dstpath), - makeCallback(callback)); + callback); }; fs.linkSync = function(srcpath, dstpath) { + nullCheck(srcpath); + nullCheck(dstpath); return binding.link(pathModule._makeLong(srcpath), pathModule._makeLong(dstpath)); }; fs.unlink = function(path, callback) { - binding.unlink(pathModule._makeLong(path), makeCallback(callback)); + callback = makeCallback(callback); + if (!nullCheck(path, callback)) return; + binding.unlink(pathModule._makeLong(path), callback); }; fs.unlinkSync = function(path) { + nullCheck(path); return binding.unlink(pathModule._makeLong(path)); }; @@ -725,12 +780,15 @@ if (constants.hasOwnProperty('O_SYMLINK')) { fs.chmod = function(path, mode, callback) { + callback = makeCallback(callback); + if (!nullCheck(path, callback)) return; binding.chmod(pathModule._makeLong(path), modeNum(mode), - makeCallback(callback)); + callback); }; fs.chmodSync = function(path, mode) { + nullCheck(path); return binding.chmod(pathModule._makeLong(path), modeNum(mode)); }; @@ -761,10 +819,13 @@ fs.fchownSync = function(fd, uid, gid) { }; fs.chown = function(path, uid, gid, callback) { - binding.chown(pathModule._makeLong(path), uid, gid, makeCallback(callback)); + callback = makeCallback(callback); + if (!nullCheck(path, callback)) return; + binding.chown(pathModule._makeLong(path), uid, gid, callback); }; fs.chownSync = function(path, uid, gid) { + nullCheck(path); return binding.chown(pathModule._makeLong(path), uid, gid); }; @@ -784,13 +845,16 @@ function toUnixTimestamp(time) { fs._toUnixTimestamp = toUnixTimestamp; fs.utimes = function(path, atime, mtime, callback) { + callback = makeCallback(callback); + if (!nullCheck(path, callback)) return; binding.utimes(pathModule._makeLong(path), toUnixTimestamp(atime), toUnixTimestamp(mtime), - makeCallback(callback)); + callback); }; fs.utimesSync = function(path, atime, mtime) { + nullCheck(path); atime = toUnixTimestamp(atime); mtime = toUnixTimestamp(mtime); binding.utimes(pathModule._makeLong(path), atime, mtime); @@ -929,6 +993,7 @@ function FSWatcher() { util.inherits(FSWatcher, EventEmitter); FSWatcher.prototype.start = function(filename, persistent) { + nullCheck(filename); var r = this._handle.start(pathModule._makeLong(filename), persistent); if (r) { @@ -942,6 +1007,7 @@ FSWatcher.prototype.close = function() { }; fs.watch = function(filename) { + nullCheck(filename); var watcher; var options; var listener; @@ -996,6 +1062,7 @@ util.inherits(StatWatcher, EventEmitter); StatWatcher.prototype.start = function(filename, persistent, interval) { + nullCheck(filename); this._handle.start(pathModule._makeLong(filename), persistent, interval); }; @@ -1013,6 +1080,7 @@ function inStatWatchers(filename) { fs.watchFile = function(filename) { + nullCheck(filename); var stat; var listener; @@ -1046,6 +1114,7 @@ fs.watchFile = function(filename) { }; fs.unwatchFile = function(filename, listener) { + nullCheck(filename); if (!inStatWatchers(filename)) return; var stat = statWatchers[filename]; diff --git a/test/simple/test-fs-null-bytes.js b/test/simple/test-fs-null-bytes.js new file mode 100644 index 00000000000..5dec223ba87 --- /dev/null +++ b/test/simple/test-fs-null-bytes.js @@ -0,0 +1,75 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +var common = require('../common'); +var assert = require('assert'); +var fs = require('fs'); + +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)); + }); + + if (sync) + assert.throws(function() { + console.error(sync.name, argsSync); + sync.apply(null, argsSync); + }, expected); + + if (async) + async.apply(null, argsAsync); +} + +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', assert.fail); +check(fs.utimes, fs.utimesSync, 'foo\u0000bar', 0, 0); +check(null, fs.watch, 'foo\u0000bar', assert.fail); +check(null, fs.watchFile, 'foo\u0000bar', assert.fail); +check(fs.writeFile, fs.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')); +