Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix --preserve-symlinks, Enhancement to exploit #10132

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 91 additions & 29 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const path = require('path');
const internalModuleReadFile = process.binding('fs').internalModuleReadFile;
const internalModuleStat = process.binding('fs').internalModuleStat;
const preserveSymlinks = !!process.binding('config').preserveSymlinks;
const adjacentNodeModules = !!process.binding('config').adjacentNodeModules;

// If obj.hasOwnProperty has been overridden, then calling
// obj.hasOwnProperty(prop) will break.
Expand Down Expand Up @@ -98,15 +99,15 @@ function readPackage(requestPath) {
return pkg;
}

function tryPackage(requestPath, exts, isMain) {
function tryPackage(requestPath, exts) {
var pkg = readPackage(requestPath);

if (!pkg) return false;

var filename = path.resolve(requestPath, pkg);
return tryFile(filename, isMain) ||
tryExtensions(filename, exts, isMain) ||
tryExtensions(path.resolve(filename, 'index'), exts, isMain);
return tryFile(filename) ||
tryExtensions(filename, exts) ||
tryExtensions(path.resolve(filename, 'index'), exts);
}

// In order to minimize unnecessary lstat() calls,
Expand All @@ -118,9 +119,9 @@ const realpathCache = new Map();
// if using --preserve-symlinks and isMain is false,
// keep symlinks intact, otherwise resolve to the
// absolute realpath.
function tryFile(requestPath, isMain) {
function tryFile(requestPath) {
const rc = stat(requestPath);
if (preserveSymlinks && !isMain) {
if (preserveSymlinks) {
return rc === 0 && path.resolve(requestPath);
}
return rc === 0 && toRealPath(requestPath);
Expand All @@ -133,9 +134,9 @@ function toRealPath(requestPath) {
}

// given a path check a the file exists with any of the set extensions
function tryExtensions(p, exts, isMain) {
function tryExtensions(p, exts) {
for (var i = 0; i < exts.length; i++) {
const filename = tryFile(p + exts[i], isMain);
const filename = tryFile(p + exts[i]);

if (filename) {
return filename;
Expand All @@ -145,7 +146,7 @@ function tryExtensions(p, exts, isMain) {
}

var warned = false;
Module._findPath = function(request, paths, isMain) {
Module._findPath = function(request, paths) {
if (path.isAbsolute(request)) {
paths = [''];
} else if (!paths || paths.length === 0) {
Expand All @@ -172,36 +173,36 @@ Module._findPath = function(request, paths, isMain) {
const rc = stat(basePath);
if (!trailingSlash) {
if (rc === 0) { // File.
if (preserveSymlinks && !isMain) {
if (preserveSymlinks) {
filename = path.resolve(basePath);
} else {
filename = toRealPath(basePath);
}
} else if (rc === 1) { // Directory.
if (exts === undefined)
exts = Object.keys(Module._extensions);
filename = tryPackage(basePath, exts, isMain);
filename = tryPackage(basePath, exts);
}

if (!filename) {
// try it with each of the extensions
if (exts === undefined)
exts = Object.keys(Module._extensions);
filename = tryExtensions(basePath, exts, isMain);
filename = tryExtensions(basePath, exts);
}
}

if (!filename && rc === 1) { // Directory.
if (exts === undefined)
exts = Object.keys(Module._extensions);
filename = tryPackage(basePath, exts, isMain);
filename = tryPackage(basePath, exts);
}

if (!filename && rc === 1) { // Directory.
// try it with each of the extensions at "index"
if (exts === undefined)
exts = Object.keys(Module._extensions);
filename = tryExtensions(path.resolve(basePath, 'index'), exts, isMain);
filename = tryExtensions(path.resolve(basePath, 'index'), exts);
}

if (filename) {
Expand Down Expand Up @@ -253,9 +254,22 @@ if (process.platform === 'win32') {
// Use colon as an extra condition since we can get node_modules
// path for dirver root like 'C:\node_modules' and don't need to
// parse driver name.
if (code === 92/*\*/ || code === 47/*/*/ || code === 58/*:*/) {
if (p !== nmLen)
paths.push(from.slice(0, last) + '\\node_modules');
if (code === 92/*\*/ || code === 47/*/*/ || code === 58/*:*/
|| (adjacentNodeModules && code === 46/*.*/)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operators go at the end of the previous line. Also, the next line should match the indentation of the start of the first argument on the previous lines.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if (p !== nmLen) {
var parent = from.slice(0, last);
paths.push(parent + '\\node_modules');

if (adjacentNodeModules) {
paths.push(parent + '+node_modules');
if (code === 46/*.*/)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Braces are missing for this conditional (even if they are syntactically optional, they're required for the project's style).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

while (i > 1) {
const code = from.charCodeAt(--i);
if (code === 92/*\*/ || code === 47/*/*/ || code === 58/*:*/)
break;
}
}
}
last = i;
p = 0;
} else if (p !== -1) {
Expand All @@ -267,6 +281,9 @@ if (process.platform === 'win32') {
}
}

// superfluous "/.node_modules"
if (adjacentNodeModules) paths.pop();

return paths;
};
} else { // posix
Expand All @@ -287,9 +304,17 @@ if (process.platform === 'win32') {
var last = from.length;
for (var i = from.length - 1; i >= 0; --i) {
const code = from.charCodeAt(i);
if (code === 47/*/*/) {
if (p !== nmLen)
paths.push(from.slice(0, last) + '/node_modules');
if (code === 47/*/*/ || (adjacentNodeModules && code === 46/*.*/)) {
if (p !== nmLen) {
var parent = from.slice(0, last);
paths.push(parent + '/node_modules');

if (adjacentNodeModules) {
paths.push(parent + '+node_modules');
if (code === 46/*.*/)
while (i > 1 && from.charCodeAt(--i) !== 47/*/*/);
}
}
last = i;
p = 0;
} else if (p !== -1) {
Expand Down Expand Up @@ -402,6 +427,34 @@ Module._resolveLookupPaths = function(request, parent) {
return [id, [path.dirname(parent.filename)]];
};

// when preserving symlinks, if the main file is both a file symlink AND
// has execute permission (on windows, its extension is in PATHEXT), then
// its target is simply read and resolved, potentially relative to the
// symlink's dirname. in all other cases the original path is preserved.
Module._resolveMainRequest = function(request) {
if(!preserveSymlinks) return request

request = Module._resolveFilename(request, null)
if(!fs.lstatSync(request).isSymbolicLink()) return request
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a space between if and ( here and all other instances.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


let isExecutable = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon here and all other instances.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if(process.platform === 'win32') {
const execExts = (process.env.PATHEXT || '').split(path.delimiter)
const requestExt = path.extname(request).toLowerCase()
isExecutable =
execExts.some(execExt => requestExt === execExt.trim().toLowerCase())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Braces should be included for arrow functions.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

} else try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing, please just include the try-catch inside a normal else {} block.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

fs.accessSync(request, fs.constants.X_OK)
isExecutable = true
} catch(e) {}

if(!isExecutable) return request

const targetPath = fs.readlinkSync(request)
return path.isAbsolute(targetPath)
? targetPath
: path.resolve(path.dirname(request), targetPath)
}

// Check the cache for the requested file.
// 1. If a module already exists in the cache: return its exports object.
Expand All @@ -415,14 +468,23 @@ Module._load = function(request, parent, isMain) {
debug('Module._load REQUEST %s parent: %s', request, parent.id);
}

var filename = Module._resolveFilename(request, parent, isMain);
var requestPath = isMain ? Module._resolveMainRequest(request) : request;
var filename = Module._resolveFilename(requestPath, parent);
var doesNonInternalExist = NativeModule.nonInternalExists(filename);

// when supporting symlinks, always cache by realpath to fix
// memory bloat and addon crashing. module's __dirname will be that
// of first requestPath that require()d it, which could be a symlink.
var cachePath = preserveSymlinks && !doesNonInternalExist
? toRealPath(filename)
: filename;

var cachedModule = Module._cache[filename];
var cachedModule = Module._cache[cachePath];
if (cachedModule) {
return cachedModule.exports;
}

if (NativeModule.nonInternalExists(filename)) {
if (doesNonInternalExist) {
debug('load native module %s', request);
return NativeModule.require(filename);
}
Expand All @@ -434,26 +496,26 @@ Module._load = function(request, parent, isMain) {
module.id = '.';
}

Module._cache[filename] = module;
Module._cache[cachePath] = module;

tryModuleLoad(module, filename);
tryModuleLoad(module, filename, cachePath);

return module.exports;
};

function tryModuleLoad(module, filename) {
function tryModuleLoad(module, filename, cachePath) {
var threw = true;
try {
module.load(filename);
threw = false;
} finally {
if (threw) {
delete Module._cache[filename];
delete Module._cache[cachePath];
}
}
}

Module._resolveFilename = function(request, parent, isMain) {
Module._resolveFilename = function(request, parent) {
if (NativeModule.nonInternalExists(request)) {
return request;
}
Expand All @@ -465,7 +527,7 @@ Module._resolveFilename = function(request, parent, isMain) {
// look up the filename first, since that's the cache key.
debug('looking for %j in %j', id, paths);

var filename = Module._findPath(request, paths, isMain);
var filename = Module._findPath(request, paths);
if (!filename) {
var err = new Error("Cannot find module '" + request + "'");
err.code = 'MODULE_NOT_FOUND';
Expand Down
10 changes: 10 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ bool trace_warnings = false;
// Used in node_config.cc to set a constant on process.binding('config')
// that is used by lib/module.js
bool config_preserve_symlinks = false;
bool config_adjacent_node_modules = false;

bool v8_initialized = false;

Expand Down Expand Up @@ -4243,6 +4244,15 @@ void Init(int* argc,
config_preserve_symlinks = (*preserve_symlinks == '1');
}

if (auto adjacent_nm = secure_getenv("NODE_ADJACENT_NODE_MODULES")) {
config_adjacent_node_modules = (*adjacent_nm == '1');
}
if (auto support_symlinks = secure_getenv("NODE_SUPPORT_SYMLINKS")) {
config_preserve_symlinks = (*support_symlinks == '1');
config_adjacent_node_modules = (*support_symlinks == '1');
}


// Parse a few arguments which are specific to Node.
int v8_argc;
const char** v8_argv;
Expand Down
3 changes: 3 additions & 0 deletions src/node_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ void InitConfig(Local<Object> target,

if (config_preserve_symlinks)
READONLY_BOOLEAN_PROPERTY("preserveSymlinks");

if (config_adjacent_node_modules)
READONLY_BOOLEAN_PROPERTY("adjacentNodeModules");
} // InitConfig

} // namespace node
Expand Down
1 change: 1 addition & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ extern const char* openssl_config;
// Used in node_config.cc to set a constant on process.binding('config')
// that is used by lib/module.js
extern bool config_preserve_symlinks;
extern bool config_adjacent_node_modules;

// Tells whether it is safe to call v8::Isolate::GetCurrent().
extern bool v8_initialized;
Expand Down