From 05802c287756e6334ac119144e5459bb32a7a52e Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 5 Aug 2022 00:41:48 +0200 Subject: [PATCH] module: protect against prototype mutation Ensures that mutating the `Object` prototype does not influence the parsing of `package.json` files. PR-URL: https://github.com/nodejs/node/pull/44007 Reviewed-By: Geoffrey Booth --- lib/internal/modules/cjs/helpers.js | 3 +- lib/internal/modules/cjs/loader.js | 21 +++++---- lib/internal/modules/esm/package_config.js | 6 ++- lib/internal/util.js | 32 ++++++++++++++ test/fixtures/es-module-specifiers/index.mjs | 3 +- .../node_modules/no-main-field/index.js | 2 + .../node_modules/no-main-field/package.json | 1 + .../test-module-prototype-mutation.js | 43 +++++++++++++++++++ 8 files changed, 96 insertions(+), 15 deletions(-) create mode 100644 test/fixtures/es-module-specifiers/node_modules/no-main-field/index.js create mode 100644 test/fixtures/es-module-specifiers/node_modules/no-main-field/package.json create mode 100644 test/parallel/test-module-prototype-mutation.js diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index ef4544ca0aaa52..20c167f8599f7b 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -24,6 +24,7 @@ const path = require('path'); const { pathToFileURL, fileURLToPath, URL } = require('internal/url'); const { getOptionValue } = require('internal/options'); +const { setOwnProperty } = require('internal/util'); const userConditions = getOptionValue('--conditions'); let debug = require('internal/util/debuglog').debuglog('module', (fn) => { @@ -117,7 +118,7 @@ function makeRequireFunction(mod, redirects) { resolve.paths = paths; - require.main = process.mainModule; + setOwnProperty(require, 'main', process.mainModule); // Enable support to add extra extension types. require.extensions = Module._extensions; diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 711589894d5d19..7c08ff8df31883 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -79,7 +79,7 @@ const { maybeCacheSourceMap, } = require('internal/source_map/source_map_cache'); const { pathToFileURL, fileURLToPath, isURLInstance } = require('internal/url'); -const { deprecate, kEmptyObject } = require('internal/util'); +const { deprecate, kEmptyObject, filterOwnProperties, setOwnProperty } = require('internal/util'); const vm = require('vm'); const assert = require('internal/assert'); const fs = require('fs'); @@ -172,7 +172,7 @@ const moduleParentCache = new SafeWeakMap(); function Module(id = '', parent) { this.id = id; this.path = path.dirname(id); - this.exports = {}; + setOwnProperty(this, 'exports', {}); moduleParentCache.set(this, parent); updateChildren(parent, this, false); this.filename = null; @@ -312,14 +312,13 @@ function readPackage(requestPath) { } try { - const parsed = JSONParse(json); - const filtered = { - name: parsed.name, - main: parsed.main, - exports: parsed.exports, - imports: parsed.imports, - type: parsed.type - }; + const filtered = filterOwnProperties(JSONParse(json), [ + 'name', + 'main', + 'exports', + 'imports', + 'type', + ]); packageJsonCache.set(jsonPath, filtered); return filtered; } catch (e) { @@ -1185,7 +1184,7 @@ Module._extensions['.json'] = function(module, filename) { } try { - module.exports = JSONParse(stripBOM(content)); + setOwnProperty(module, 'exports', JSONParse(stripBOM(content))); } catch (err) { err.message = filename + ': ' + err.message; throw err; diff --git a/lib/internal/modules/esm/package_config.js b/lib/internal/modules/esm/package_config.js index 89e90d0d997cd2..7271d065de9461 100644 --- a/lib/internal/modules/esm/package_config.js +++ b/lib/internal/modules/esm/package_config.js @@ -2,6 +2,7 @@ const { JSONParse, + ObjectPrototypeHasOwnProperty, SafeMap, StringPrototypeEndsWith, } = primordials; @@ -11,6 +12,7 @@ const { } = require('internal/errors').codes; const packageJsonReader = require('internal/modules/package_json_reader'); +const { filterOwnProperties } = require('internal/util'); /** @@ -66,8 +68,8 @@ function getPackageConfig(path, specifier, base) { ); } - let { imports, main, name, type } = packageJSON; - const { exports } = packageJSON; + let { imports, main, name, type } = filterOwnProperties(packageJSON, ['imports', 'main', 'name', 'type']); + const exports = ObjectPrototypeHasOwnProperty(packageJSON, 'exports') ? packageJSON.exports : undefined; if (typeof imports !== 'object' || imports === null) { imports = undefined; } diff --git a/lib/internal/util.js b/lib/internal/util.js index 82b52b401984fb..c077f4be2334a5 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -14,6 +14,7 @@ const { ObjectGetOwnPropertyDescriptors, ObjectGetPrototypeOf, ObjectFreeze, + ObjectPrototypeHasOwnProperty, ObjectSetPrototypeOf, Promise, ReflectApply, @@ -507,6 +508,35 @@ ObjectFreeze(kEnumerableProperty); const kEmptyObject = ObjectFreeze(ObjectCreate(null)); +function filterOwnProperties(source, keys) { + const filtered = ObjectCreate(null); + for (let i = 0; i < keys.length; i++) { + const key = keys[i]; + if (ObjectPrototypeHasOwnProperty(source, key)) { + filtered[key] = source[key]; + } + } + + return filtered; +} + +/** + * Mimics `obj[key] = value` but ignoring potential prototype inheritance. + * @param {any} obj + * @param {string} key + * @param {any} value + * @returns {any} + */ +function setOwnProperty(obj, key, value) { + return ObjectDefineProperty(obj, key, { + __proto__: null, + configurable: true, + enumerable: true, + value, + writable: true, + }); +} + module.exports = { assertCrypto, cachedResult, @@ -519,6 +549,7 @@ module.exports = { emitExperimentalWarning, exposeInterface, filterDuplicateStrings, + filterOwnProperties, getConstructorOf, getSystemErrorMap, getSystemErrorName, @@ -549,4 +580,5 @@ module.exports = { kEmptyObject, kEnumerableProperty, + setOwnProperty, }; diff --git a/test/fixtures/es-module-specifiers/index.mjs b/test/fixtures/es-module-specifiers/index.mjs index 2be7048513f287..8c361af157b70c 100644 --- a/test/fixtures/es-module-specifiers/index.mjs +++ b/test/fixtures/es-module-specifiers/index.mjs @@ -1,10 +1,11 @@ import explicit from 'explicit-main'; import implicit from 'implicit-main'; import implicitModule from 'implicit-main-type-module'; +import noMain from 'no-main-field'; function getImplicitCommonjs () { return import('implicit-main-type-commonjs'); } -export {explicit, implicit, implicitModule, getImplicitCommonjs}; +export {explicit, implicit, implicitModule, getImplicitCommonjs, noMain}; export default 'success'; diff --git a/test/fixtures/es-module-specifiers/node_modules/no-main-field/index.js b/test/fixtures/es-module-specifiers/node_modules/no-main-field/index.js new file mode 100644 index 00000000000000..528a6ff2acbf84 --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/no-main-field/index.js @@ -0,0 +1,2 @@ +'use strict'; +module.exports = 'no main field'; diff --git a/test/fixtures/es-module-specifiers/node_modules/no-main-field/package.json b/test/fixtures/es-module-specifiers/node_modules/no-main-field/package.json new file mode 100644 index 00000000000000..0967ef424bce67 --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/no-main-field/package.json @@ -0,0 +1 @@ +{} diff --git a/test/parallel/test-module-prototype-mutation.js b/test/parallel/test-module-prototype-mutation.js new file mode 100644 index 00000000000000..b21dba87b6aa40 --- /dev/null +++ b/test/parallel/test-module-prototype-mutation.js @@ -0,0 +1,43 @@ +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); + +Object.defineProperty(Object.prototype, 'name', { + __proto__: null, + get: common.mustNotCall('get %Object.prototype%.name'), + set: common.mustNotCall('set %Object.prototype%.name'), + enumerable: false, +}); +Object.defineProperty(Object.prototype, 'main', { + __proto__: null, + get: common.mustNotCall('get %Object.prototype%.main'), + set: common.mustNotCall('set %Object.prototype%.main'), + enumerable: false, +}); +Object.defineProperty(Object.prototype, 'type', { + __proto__: null, + get: common.mustNotCall('get %Object.prototype%.type'), + set: common.mustNotCall('set %Object.prototype%.type'), + enumerable: false, +}); +Object.defineProperty(Object.prototype, 'exports', { + __proto__: null, + get: common.mustNotCall('get %Object.prototype%.exports'), + set: common.mustNotCall('set %Object.prototype%.exports'), + enumerable: false, +}); +Object.defineProperty(Object.prototype, 'imports', { + __proto__: null, + get: common.mustNotCall('get %Object.prototype%.imports'), + set: common.mustNotCall('set %Object.prototype%.imports'), + enumerable: false, +}); + +assert.strictEqual( + require(fixtures.path('es-module-specifiers', 'node_modules', 'no-main-field')), + 'no main field' +); + +import(fixtures.fileURL('es-module-specifiers', 'index.mjs')) + .then(common.mustCall((module) => assert.strictEqual(module.noMain, 'no main field')));