Skip to content

Commit fa115ee

Browse files
aduh95richardlau
aduh95
authored and
richardlau
committed
module: protect against prototype mutation
Ensures that mutating the `Object` prototype does not influence the parsing of `package.json` files. Backport-PR-URL: nodejs-private/node-private#373 PR-URL: #44007 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
1 parent 97a0443 commit fa115ee

File tree

15 files changed

+77
-23
lines changed

15 files changed

+77
-23
lines changed

‎lib/internal/modules/cjs/helpers.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const path = require('path');
2323
const { pathToFileURL, fileURLToPath, URL } = require('internal/url');
2424

2525
const { getOptionValue } = require('internal/options');
26+
const { setOwnProperty } = require('internal/util');
2627
const userConditions = getOptionValue('--conditions');
2728

2829
let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
@@ -116,7 +117,7 @@ function makeRequireFunction(mod, redirects) {
116117

117118
resolve.paths = paths;
118119

119-
require.main = process.mainModule;
120+
setOwnProperty(require, 'main', process.mainModule);
120121

121122
// Enable support to add extra extension types.
122123
require.extensions = Module._extensions;

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ const {
7676
maybeCacheSourceMap,
7777
} = require('internal/source_map/source_map_cache');
7878
const { pathToFileURL, fileURLToPath, isURLInstance } = require('internal/url');
79-
const { deprecate } = require('internal/util');
79+
const { deprecate, filterOwnProperties, setOwnProperty } = require('internal/util');
8080
const vm = require('vm');
8181
const assert = require('internal/assert');
8282
const fs = require('fs');
@@ -163,7 +163,7 @@ function updateChildren(parent, child, scan) {
163163
function Module(id = '', parent) {
164164
this.id = id;
165165
this.path = path.dirname(id);
166-
this.exports = {};
166+
setOwnProperty(this, 'exports', {});
167167
this.parent = parent;
168168
updateChildren(parent, this, false);
169169
this.filename = null;
@@ -269,14 +269,13 @@ function readPackage(requestPath) {
269269
}
270270

271271
try {
272-
const parsed = JSONParse(json);
273-
const filtered = {
274-
name: parsed.name,
275-
main: parsed.main,
276-
exports: parsed.exports,
277-
imports: parsed.imports,
278-
type: parsed.type
279-
};
272+
const filtered = filterOwnProperties(JSONParse(json), [
273+
'name',
274+
'main',
275+
'exports',
276+
'imports',
277+
'type',
278+
]);
280279
packageJsonCache.set(jsonPath, filtered);
281280
return filtered;
282281
} catch (e) {
@@ -1125,7 +1124,7 @@ Module._extensions['.json'] = function(module, filename) {
11251124
}
11261125

11271126
try {
1128-
module.exports = JSONParse(stripBOM(content));
1127+
setOwnProperty(module, 'exports', JSONParse(stripBOM(content)));
11291128
} catch (err) {
11301129
err.message = filename + ': ' + err.message;
11311130
throw err;

‎lib/internal/util.js‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
ObjectGetOwnPropertyDescriptor,
1212
ObjectGetOwnPropertyDescriptors,
1313
ObjectGetPrototypeOf,
14+
ObjectPrototypeHasOwnProperty,
1415
ObjectSetPrototypeOf,
1516
Promise,
1617
ReflectConstruct,
@@ -458,6 +459,35 @@ function createDeferredPromise() {
458459
return { promise, resolve, reject };
459460
}
460461

462+
function filterOwnProperties(source, keys) {
463+
const filtered = ObjectCreate(null);
464+
for (let i = 0; i < keys.length; i++) {
465+
const key = keys[i];
466+
if (ObjectPrototypeHasOwnProperty(source, key)) {
467+
filtered[key] = source[key];
468+
}
469+
}
470+
471+
return filtered;
472+
}
473+
474+
/**
475+
* Mimics `obj[key] = value` but ignoring potential prototype inheritance.
476+
* @param {any} obj
477+
* @param {string} key
478+
* @param {any} value
479+
* @returns {any}
480+
*/
481+
function setOwnProperty(obj, key, value) {
482+
return ObjectDefineProperty(obj, key, {
483+
__proto__: null,
484+
configurable: true,
485+
enumerable: true,
486+
value,
487+
writable: true,
488+
});
489+
}
490+
461491
module.exports = {
462492
assertCrypto,
463493
cachedResult,
@@ -468,6 +498,7 @@ module.exports = {
468498
deprecate,
469499
emitExperimentalWarning,
470500
filterDuplicateStrings,
501+
filterOwnProperties,
471502
getConstructorOf,
472503
getSystemErrorMap,
473504
getSystemErrorName,
@@ -492,5 +523,6 @@ module.exports = {
492523
// Used by the buffer module to capture an internal reference to the
493524
// default isEncoding implementation, just in case userland overrides it.
494525
kIsEncodingSymbol: Symbol('kIsEncodingSymbol'),
495-
kVmBreakFirstLineSymbol: Symbol('kVmBreakFirstLineSymbol')
526+
kVmBreakFirstLineSymbol: Symbol('kVmBreakFirstLineSymbol'),
527+
setOwnProperty,
496528
};

‎test/common/fixtures.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,18 @@
22

33
const path = require('path');
44
const fs = require('fs');
5+
const { pathToFileURL } = require('url');
56

67
const fixturesDir = path.join(__dirname, '..', 'fixtures');
78

89
function fixturesPath(...args) {
910
return path.join(fixturesDir, ...args);
1011
}
1112

13+
function fixturesFileURL(...args) {
14+
return pathToFileURL(fixturesPath(...args));
15+
}
16+
1217
function readFixtureSync(args, enc) {
1318
if (Array.isArray(args))
1419
return fs.readFileSync(fixturesPath(...args), enc);
@@ -22,6 +27,7 @@ function readFixtureKey(name, enc) {
2227
module.exports = {
2328
fixturesDir,
2429
path: fixturesPath,
30+
fileURL: fixturesFileURL,
2531
readSync: readFixtureSync,
2632
readKey: readFixtureKey
2733
};

‎test/fixtures/es-module-specifiers/index.mjs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import explicit from 'explicit-main';
22
import implicit from 'implicit-main';
33
import implicitModule from 'implicit-main-type-module';
4+
import noMain from 'no-main-field';
45

56
function getImplicitCommonjs () {
67
return import('implicit-main-type-commonjs');
78
}
89

9-
export {explicit, implicit, implicitModule, getImplicitCommonjs};
10+
export {explicit, implicit, implicitModule, getImplicitCommonjs, noMain};
1011
export default 'success';

‎test/fixtures/es-module-specifiers/node_modules/no-main-field/index.js‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/fixtures/es-module-specifiers/node_modules/no-main-field/package.json‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/message/source_map_disabled_by_api.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Error: an error!
88
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
99
at Module.load (internal/modules/cjs/loader.js:*)
1010
at Function.Module._load (internal/modules/cjs/loader.js:*)
11-
at Module.require (internal/modules/cjs/loader.js:*)
11+
at Module.internalRequire (internal/modules/cjs/loader.js:*)
1212
*enclosing-call-site.js:16
1313
throw new Error('an error!')
1414
^
@@ -23,4 +23,4 @@ Error: an error!
2323
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
2424
at Module.load (internal/modules/cjs/loader.js:*)
2525
at Function.Module._load (internal/modules/cjs/loader.js:*)
26-
at Module.require (internal/modules/cjs/loader.js:*)
26+
at Module.internalRequire (internal/modules/cjs/loader.js:*)

‎test/message/source_map_enabled_by_api.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Error: an error!
1212
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
1313
at Module.load (internal/modules/cjs/loader.js:*)
1414
at Function.Module._load (internal/modules/cjs/loader.js:*)
15-
at Module.require (internal/modules/cjs/loader.js:*)
15+
at Module.internalRequire (internal/modules/cjs/loader.js:*)
1616
*enclosing-call-site-min.js:1
1717
var functionA=function(){functionB()};function functionB(){functionC()}var functionC=function(){functionD()},functionD=function(){if(0<Math.random())throw Error("an error!");},thrower=functionA;try{functionA()}catch(a){throw a;};
1818
^
@@ -27,4 +27,4 @@ Error: an error!
2727
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
2828
at Module.load (internal/modules/cjs/loader.js:*)
2929
at Function.Module._load (internal/modules/cjs/loader.js:*)
30-
at Module.require (internal/modules/cjs/loader.js:*)
30+
at Module.internalRequire (internal/modules/cjs/loader.js:*)

‎test/message/source_map_enclosing_function.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ Error: an error!
1212
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
1313
at Module.load (internal/modules/cjs/loader.js:*)
1414
at Function.Module._load (internal/modules/cjs/loader.js:*)
15-
at Module.require (internal/modules/cjs/loader.js:*)
15+
at Module.internalRequire (internal/modules/cjs/loader.js:*)

0 commit comments

Comments
 (0)