Skip to content

Commit

Permalink
fs: validate args of truncate functions in js
Browse files Browse the repository at this point in the history 
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: nodejs#2498
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
@thefourtheye
thefourtheye committed Jul 21, 2016
1 parent 9359de9 commit c86c1ee
Show file tree
Hide file tree
Showing 6 changed files with 301 additions and 118 deletions.
107 changes: 70 additions & 37 deletions lib/fs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 ' +
Expand Down Expand Up @@ -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' +
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -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);
};

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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')) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit c86c1ee

Please sign in to comment.