diff options
author | Antoine du Hamel <duhamelantoine1995@gmail.com> | 2022-08-05 00:41:48 +0200 |
---|---|---|
committer | Richard Lau <rlau@redhat.com> | 2023-02-15 13:31:05 +0000 |
commit | fa115ee8ac32883c96df4a93fafb600bd665a5aa (patch) | |
tree | 519f24011b91b7e37eb6efe747900807831df03a | |
parent | 97a0443f1369e65cf656a529b2f5433bfd56ad92 (diff) | |
download | node-new-fa115ee8ac32883c96df4a93fafb600bd665a5aa.tar.gz |
module: protect against prototype mutation
Ensures that mutating the `Object` prototype does not influence the
parsing of `package.json` files.
Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/373
PR-URL: https://github.com/nodejs/node/pull/44007
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
-rw-r--r-- | lib/internal/modules/cjs/helpers.js | 3 | ||||
-rw-r--r-- | lib/internal/modules/cjs/loader.js | 21 | ||||
-rw-r--r-- | lib/internal/util.js | 34 | ||||
-rw-r--r-- | test/common/fixtures.js | 6 | ||||
-rw-r--r-- | test/fixtures/es-module-specifiers/index.mjs | 3 | ||||
-rw-r--r-- | test/fixtures/es-module-specifiers/node_modules/no-main-field/index.js | 2 | ||||
-rw-r--r-- | test/fixtures/es-module-specifiers/node_modules/no-main-field/package.json | 1 | ||||
-rw-r--r-- | test/message/source_map_disabled_by_api.out | 4 | ||||
-rw-r--r-- | test/message/source_map_enabled_by_api.out | 4 | ||||
-rw-r--r-- | test/message/source_map_enclosing_function.out | 2 | ||||
-rw-r--r-- | test/message/source_map_reference_error_tabs.out | 2 | ||||
-rw-r--r-- | test/message/source_map_throw_catch.out | 2 | ||||
-rw-r--r-- | test/message/source_map_throw_first_tick.out | 2 | ||||
-rw-r--r-- | test/message/source_map_throw_icu.out | 2 | ||||
-rw-r--r-- | test/parallel/test-module-prototype-mutation.js | 12 |
15 files changed, 77 insertions, 23 deletions
diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index 8a703c46ce..7fed1a1837 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -23,6 +23,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) => { @@ -116,7 +117,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 92071f641a..5e61ae80ce 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -76,7 +76,7 @@ const { maybeCacheSourceMap, } = require('internal/source_map/source_map_cache'); const { pathToFileURL, fileURLToPath, isURLInstance } = require('internal/url'); -const { deprecate } = require('internal/util'); +const { deprecate, filterOwnProperties, setOwnProperty } = require('internal/util'); const vm = require('vm'); const assert = require('internal/assert'); const fs = require('fs'); @@ -163,7 +163,7 @@ function updateChildren(parent, child, scan) { function Module(id = '', parent) { this.id = id; this.path = path.dirname(id); - this.exports = {}; + setOwnProperty(this, 'exports', {}); this.parent = parent; updateChildren(parent, this, false); this.filename = null; @@ -269,14 +269,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) { @@ -1125,7 +1124,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/util.js b/lib/internal/util.js index f8cbac7ec7..60fbf199e6 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -11,6 +11,7 @@ const { ObjectGetOwnPropertyDescriptor, ObjectGetOwnPropertyDescriptors, ObjectGetPrototypeOf, + ObjectPrototypeHasOwnProperty, ObjectSetPrototypeOf, Promise, ReflectConstruct, @@ -458,6 +459,35 @@ function createDeferredPromise() { return { promise, resolve, reject }; } +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, @@ -468,6 +498,7 @@ module.exports = { deprecate, emitExperimentalWarning, filterDuplicateStrings, + filterOwnProperties, getConstructorOf, getSystemErrorMap, getSystemErrorName, @@ -492,5 +523,6 @@ module.exports = { // Used by the buffer module to capture an internal reference to the // default isEncoding implementation, just in case userland overrides it. kIsEncodingSymbol: Symbol('kIsEncodingSymbol'), - kVmBreakFirstLineSymbol: Symbol('kVmBreakFirstLineSymbol') + kVmBreakFirstLineSymbol: Symbol('kVmBreakFirstLineSymbol'), + setOwnProperty, }; diff --git a/test/common/fixtures.js b/test/common/fixtures.js index 76e72a22ec..75df77c1bb 100644 --- a/test/common/fixtures.js +++ b/test/common/fixtures.js @@ -2,6 +2,7 @@ const path = require('path'); const fs = require('fs'); +const { pathToFileURL } = require('url'); const fixturesDir = path.join(__dirname, '..', 'fixtures'); @@ -9,6 +10,10 @@ function fixturesPath(...args) { return path.join(fixturesDir, ...args); } +function fixturesFileURL(...args) { + return pathToFileURL(fixturesPath(...args)); +} + function readFixtureSync(args, enc) { if (Array.isArray(args)) return fs.readFileSync(fixturesPath(...args), enc); @@ -22,6 +27,7 @@ function readFixtureKey(name, enc) { module.exports = { fixturesDir, path: fixturesPath, + fileURL: fixturesFileURL, readSync: readFixtureSync, readKey: readFixtureKey }; diff --git a/test/fixtures/es-module-specifiers/index.mjs b/test/fixtures/es-module-specifiers/index.mjs index 2be7048513..8c361af157 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 0000000000..528a6ff2ac --- /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 0000000000..0967ef424b --- /dev/null +++ b/test/fixtures/es-module-specifiers/node_modules/no-main-field/package.json @@ -0,0 +1 @@ +{} diff --git a/test/message/source_map_disabled_by_api.out b/test/message/source_map_disabled_by_api.out index 46762a0b47..a0c4f6d5cb 100644 --- a/test/message/source_map_disabled_by_api.out +++ b/test/message/source_map_disabled_by_api.out @@ -8,7 +8,7 @@ Error: an error! at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.require (internal/modules/cjs/loader.js:*) + at Module.internalRequire (internal/modules/cjs/loader.js:*) *enclosing-call-site.js:16 throw new Error('an error!') ^ @@ -23,4 +23,4 @@ Error: an error! at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.require (internal/modules/cjs/loader.js:*) + at Module.internalRequire (internal/modules/cjs/loader.js:*) diff --git a/test/message/source_map_enabled_by_api.out b/test/message/source_map_enabled_by_api.out index 3f111ee2f7..a5d53028d6 100644 --- a/test/message/source_map_enabled_by_api.out +++ b/test/message/source_map_enabled_by_api.out @@ -12,7 +12,7 @@ Error: an error! at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.require (internal/modules/cjs/loader.js:*) + at Module.internalRequire (internal/modules/cjs/loader.js:*) *enclosing-call-site-min.js:1 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;}; ^ @@ -27,4 +27,4 @@ Error: an error! at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.require (internal/modules/cjs/loader.js:*) + at Module.internalRequire (internal/modules/cjs/loader.js:*) diff --git a/test/message/source_map_enclosing_function.out b/test/message/source_map_enclosing_function.out index 057c1e7629..a74ecff730 100644 --- a/test/message/source_map_enclosing_function.out +++ b/test/message/source_map_enclosing_function.out @@ -12,4 +12,4 @@ Error: an error! at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.require (internal/modules/cjs/loader.js:*) + at Module.internalRequire (internal/modules/cjs/loader.js:*) diff --git a/test/message/source_map_reference_error_tabs.out b/test/message/source_map_reference_error_tabs.out index 275c703c66..07c54000d1 100644 --- a/test/message/source_map_reference_error_tabs.out +++ b/test/message/source_map_reference_error_tabs.out @@ -9,7 +9,7 @@ ReferenceError: alert is not defined at Object.Module._extensions..js (internal/modules/cjs/loader.js:* at Module.load (internal/modules/cjs/loader.js:* at Function.Module._load (internal/modules/cjs/loader.js:* - at Module.require (internal/modules/cjs/loader.js:* + at Module.internalRequire (internal/modules/cjs/loader.js:* at require (internal/modules/cjs/helpers.js:* at Object.<anonymous> (*source_map_reference_error_tabs.js:* at Module._compile (internal/modules/cjs/loader.js:* diff --git a/test/message/source_map_throw_catch.out b/test/message/source_map_throw_catch.out index b2123e6eeb..a228d5f3ea 100644 --- a/test/message/source_map_throw_catch.out +++ b/test/message/source_map_throw_catch.out @@ -9,7 +9,7 @@ Error: an exception at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.require (internal/modules/cjs/loader.js:*) + at Module.internalRequire (internal/modules/cjs/loader.js:*) at require (internal/modules/cjs/helpers.js:*) at Object.<anonymous> (*source_map_throw_catch.js:6:3) at Module._compile (internal/modules/cjs/loader.js:*) diff --git a/test/message/source_map_throw_first_tick.out b/test/message/source_map_throw_first_tick.out index e4a8e9ff25..b99da0283c 100644 --- a/test/message/source_map_throw_first_tick.out +++ b/test/message/source_map_throw_first_tick.out @@ -9,7 +9,7 @@ Error: an exception at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.require (internal/modules/cjs/loader.js:*) + at Module.internalRequire (internal/modules/cjs/loader.js:*) at require (internal/modules/cjs/helpers.js:*) at Object.<anonymous> (*source_map_throw_first_tick.js:5:1) at Module._compile (internal/modules/cjs/loader.js:*) diff --git a/test/message/source_map_throw_icu.out b/test/message/source_map_throw_icu.out index 9ceb20125f..c4e7e32c2b 100644 --- a/test/message/source_map_throw_icu.out +++ b/test/message/source_map_throw_icu.out @@ -9,7 +9,7 @@ Error: an error at Object.Module._extensions..js (internal/modules/cjs/loader.js:* at Module.load (internal/modules/cjs/loader.js:* at Function.Module._load (internal/modules/cjs/loader.js:* - at Module.require (internal/modules/cjs/loader.js:* + at Module.internalRequire (internal/modules/cjs/loader.js:* at require (internal/modules/cjs/helpers.js:* at Object.<anonymous> (*source_map_throw_icu.js:* at Module._compile (internal/modules/cjs/loader.js:* diff --git a/test/parallel/test-module-prototype-mutation.js b/test/parallel/test-module-prototype-mutation.js new file mode 100644 index 0000000000..9e50f91a7f --- /dev/null +++ b/test/parallel/test-module-prototype-mutation.js @@ -0,0 +1,12 @@ +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); + +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'))); |