fs: validate args of truncate functions in js

This patch

 1. moves the basic validation of arguments to `truncate` family
    of functions to the JavaScript layer from the C++ layer.

 2. makes sure that the File Descriptors are validated strictly.

PR-URL: #2498
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

Author: thefourtheye

lib/fs.js

@@ -81,6 +81,16 @@ function throwOptionsError(options) {
     'but got ' + typeof options + ' instead');
 }
 
+function isFD(fd) {
+  return Number.isInteger(fd) && fd >= 0 && fd <= 0xFFFFFFFF;
+}
+
+function sanitizeFD(fd) {
+  if (!isFD(fd))
+    throw new TypeError('file descriptor must be a unsigned 32-bit integer');
+  return fd;
+}
+
 // Ensure that callbacks run in the global context. Only use this function
 // for callbacks that are passed to the binding layer, callbacks that are
 // invoked from JS already run in the proper scope.
@@ -112,10 +122,6 @@ function nullCheck(path, callback) {
   return true;
 }
 
-function isFd(path) {
-  return (path >>> 0) === path;
-}
-
 // Static method to set the stats properties on a Stats object.
 fs.Stats = function(
     dev,
@@ -257,7 +263,7 @@ fs.readFile = function(path, options, callback) {
     return;
 
   var context = new ReadFileContext(callback, encoding);
-  context.isUserFd = isFd(path); // file descriptor ownership
+  context.isUserFd = isFD(path); // file descriptor ownership
   var req = new FSReqWrap();
   req.context = context;
   req.oncomplete = readFileAfterOpen;
@@ -473,7 +479,7 @@ fs.readFileSync = function(path, options) {
   assertEncoding(encoding);
 
   var flag = options.flag || 'r';
-  var isUserFd = isFd(path); // file descriptor ownership
+  var isUserFd = isFD(path); // file descriptor ownership
   var fd = isUserFd ? path : fs.openSync(path, flag, 0o666);
 
   var st = tryStatSync(fd, isUserFd);
@@ -570,12 +576,12 @@ Object.defineProperty(exports, '_stringToFlags', {
 
 fs.close = function(fd, callback) {
   var req = new FSReqWrap();
-  req.oncomplete = makeCallback(arguments[arguments.length - 1]);
-  binding.close(fd, req);
+  req.oncomplete = makeCallback(callback);
+  binding.close(sanitizeFD(fd), req);
 };
 
 fs.closeSync = function(fd) {
-  return binding.close(fd);
+  return binding.close(sanitizeFD(fd));
 };
 
 function modeNum(m, def) {
@@ -612,6 +618,7 @@ fs.openSync = function(path, flags, mode) {
 var readWarned = false;
 fs.read = function(fd, buffer, offset, length, position, callback) {
   callback = makeCallback(arguments[arguments.length - 1]);
+  fd = sanitizeFD(fd);
   if (!(buffer instanceof Buffer)) {
     // legacy string interface (fd, length, position, encoding, callback)
     readWarned = printDeprecation('fs.read\'s legacy String interface ' +
@@ -672,6 +679,7 @@ fs.readSync = function(fd, buffer, offset, length, position) {
   var legacy = false;
   var encoding;
 
+  fd = sanitizeFD(fd);
   if (!(buffer instanceof Buffer)) {
     // legacy string interface (fd, length, position, encoding, callback)
     readSyncWarned = printDeprecation('fs.readSync\'s legacy String interface' +
@@ -713,6 +721,7 @@ fs.readSync = function(fd, buffer, offset, length, position) {
 //  fs.write(fd, string[, position[, encoding]], callback);
 fs.write = function(fd, buffer, offset, length, position, callback) {
   callback = makeCallback(arguments[arguments.length - 1]);
+  fd = sanitizeFD(fd);
   function wrapper(err, written) {
     // Retain a reference to buffer so that it can't be GC'ed too soon.
     callback(err, written || 0, buffer);
@@ -748,6 +757,7 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
 // OR
 //  fs.writeSync(fd, string[, position[, encoding]]);
 fs.writeSync = function(fd, buffer, offset, length, position) {
+  fd = sanitizeFD(fd);
   if (buffer instanceof Buffer) {
     if (position === undefined)
       position = null;
@@ -779,14 +789,18 @@ fs.renameSync = function(oldPath, newPath) {
 };
 
 fs.truncate = function(path, len, callback) {
-  if (typeof path === 'number') {
+  callback = makeCallback(arguments[arguments.length - 1]);
+
+  if (isFD(path))
     return fs.ftruncate(path, len, callback);
-  }
 
-  callback = makeCallback(arguments[arguments.length - 1]);
+  if (typeof path !== 'string')
+    throw new TypeError('path must be a string');
 
-  if (typeof len === 'function' || len === undefined) {
+  if (typeof len === 'function' || len == undefined) {
     len = 0;
+  } else if (!Number.isInteger(len) || len < 0) {
+    throw new TypeError('length must be a positive integer');
   }
 
   fs.open(path, 'r+', function(er, fd) {
@@ -802,13 +816,18 @@ fs.truncate = function(path, len, callback) {
 };
 
 fs.truncateSync = function(path, len) {
-  if (typeof path === 'number') {
-    // legacy
+  if (isFD(path))
     return fs.ftruncateSync(path, len);
-  }
-  if (len === undefined) {
+
+  if (typeof path !== 'string')
+    throw new TypeError('path must be a string');
+
+  if (len === undefined || len === null) {
     len = 0;
+  } else if (!Number.isInteger(len) || len < 0) {
+    throw new TypeError('length must be a positive integer');
   }
+
   // allow error to be thrown, but still close fd.
   var fd = fs.openSync(path, 'r+');
   var ret;
@@ -822,18 +841,30 @@ fs.truncateSync = function(path, len) {
 };
 
 fs.ftruncate = function(fd, len, callback) {
-  if (typeof len === 'function' || len === undefined) {
+  callback = makeCallback(arguments[arguments.length - 1]);
+
+  fd = sanitizeFD(fd);
+
+  if (typeof len === 'function' || len == undefined) {
     len = 0;
+  } else if (!Number.isInteger(len) || len < 0) {
+    throw new TypeError('length must be a positive integer');
   }
+
   var req = new FSReqWrap();
-  req.oncomplete = makeCallback(arguments[arguments.length - 1]);
+  req.oncomplete = callback;
   binding.ftruncate(fd, len, req);
 };
 
 fs.ftruncateSync = function(fd, len) {
-  if (len === undefined) {
+  fd = sanitizeFD(fd);
+
+  if (len === undefined || len === null) {
     len = 0;
+  } else if (!Number.isInteger(len) || len < 0) {
+    throw new TypeError('length must be a positive integer');
   }
+
   return binding.ftruncate(fd, len);
 };
 
@@ -853,21 +884,21 @@ fs.rmdirSync = function(path) {
 fs.fdatasync = function(fd, callback) {
   var req = new FSReqWrap();
   req.oncomplete = makeCallback(callback);
-  binding.fdatasync(fd, req);
+  binding.fdatasync(sanitizeFD(fd), req);
 };
 
 fs.fdatasyncSync = function(fd) {
-  return binding.fdatasync(fd);
+  return binding.fdatasync(sanitizeFD(fd));
 };
 
 fs.fsync = function(fd, callback) {
   var req = new FSReqWrap();
-  req.oncomplete = makeCallback(arguments[arguments.length - 1]);
-  binding.fsync(fd, req);
+  req.oncomplete = makeCallback(callback);
+  binding.fsync(sanitizeFD(fd), req);
 };
 
 fs.fsyncSync = function(fd) {
-  return binding.fsync(fd);
+  return binding.fsync(sanitizeFD(fd));
 };
 
 fs.mkdir = function(path, mode, callback) {
@@ -915,8 +946,8 @@ fs.readdirSync = function(path, options) {
 
 fs.fstat = function(fd, callback) {
   var req = new FSReqWrap();
-  req.oncomplete = makeCallback(arguments[arguments.length - 1]);
-  binding.fstat(fd, req);
+  req.oncomplete = makeCallback(callback);
+  binding.fstat(sanitizeFD(fd), req);
 };
 
 fs.lstat = function(path, callback) {
@@ -936,7 +967,7 @@ fs.stat = function(path, callback) {
 };
 
 fs.fstatSync = function(fd) {
-  return binding.fstat(fd);
+  return binding.fstat(sanitizeFD(fd));
 };
 
 fs.lstatSync = function(path) {
@@ -1053,11 +1084,11 @@ fs.unlinkSync = function(path) {
 fs.fchmod = function(fd, mode, callback) {
   var req = new FSReqWrap();
   req.oncomplete = makeCallback(arguments[arguments.length - 1]);
-  binding.fchmod(fd, modeNum(mode), req);
+  binding.fchmod(sanitizeFD(fd), modeNum(mode), req);
 };
 
 fs.fchmodSync = function(fd, mode) {
-  return binding.fchmod(fd, modeNum(mode));
+  return binding.fchmod(sanitizeFD(fd), modeNum(mode));
 };
 
 if (constants.hasOwnProperty('O_SYMLINK')) {
@@ -1136,11 +1167,11 @@ if (constants.hasOwnProperty('O_SYMLINK')) {
 fs.fchown = function(fd, uid, gid, callback) {
   var req = new FSReqWrap();
   req.oncomplete = makeCallback(arguments[arguments.length - 1]);
-  binding.fchown(fd, uid, gid, req);
+  binding.fchown(sanitizeFD(fd), uid, gid, req);
 };
 
 fs.fchownSync = function(fd, uid, gid) {
-  return binding.fchown(fd, uid, gid);
+  return binding.fchown(sanitizeFD(fd), uid, gid);
 };
 
 fs.chown = function(path, uid, gid, callback) {
@@ -1197,6 +1228,7 @@ fs.utimesSync = function(path, atime, mtime) {
 
 fs.futimes = function(fd, atime, mtime, callback) {
   callback = makeCallback(arguments[arguments.length - 1]);
+  fd = sanitizeFD(fd);
   atime = toUnixTimestamp(atime);
   mtime = toUnixTimestamp(mtime);
   var req = new FSReqWrap();
@@ -1205,6 +1237,7 @@ fs.futimes = function(fd, atime, mtime, callback) {
 };
 
 fs.futimesSync = function(fd, atime, mtime) {
+  fd = sanitizeFD(fd);
   atime = toUnixTimestamp(atime);
   mtime = toUnixTimestamp(mtime);
   binding.futimes(fd, atime, mtime);
@@ -1257,7 +1290,7 @@ fs.writeFile = function(path, data, options, callback) {
 
   var flag = options.flag || 'w';
 
-  if (isFd(path)) {
+  if (isFD(path)) {
     writeFd(path, true);
     return;
   }
@@ -1291,7 +1324,7 @@ fs.writeFileSync = function(path, data, options) {
   assertEncoding(options.encoding);
 
   var flag = options.flag || 'w';
-  var isUserFd = isFd(path); // file descriptor ownership
+  var isUserFd = isFD(path); // file descriptor ownership
   var fd = isUserFd ? path : fs.openSync(path, flag, options.mode);
 
   if (!(data instanceof Buffer)) {
@@ -1329,7 +1362,7 @@ fs.appendFile = function(path, data, options, callback) {
     options = util._extend({ flag: 'a' }, options);
 
   // force append behavior when using a supplied file descriptor
-  if (isFd(path))
+  if (isFD(path))
     options.flag = 'a';
 
   fs.writeFile(path, data, options, callback);
@@ -1348,7 +1381,7 @@ fs.appendFileSync = function(path, data, options) {
     options = util._extend({ flag: 'a' }, options);
 
   // force append behavior when using a supplied file descriptor
-  if (isFd(path))
+  if (isFD(path))
     options.flag = 'a';
 
   fs.writeFileSync(path, data, options);
@@ -1932,7 +1965,7 @@ function SyncWriteStream(fd, options) {
 
   options = options || {};
 
-  this.fd = fd;
+  this.fd = sanitizeFD(fd);
   this.writable = true;
   this.readable = false;
   this.autoClose = options.autoClose === undefined ? true : options.autoClose;