Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
devel:languages:nodejs
nodejs12
CVE-2023-23918.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File CVE-2023-23918.patch of Package nodejs12
commit f7892c16be6507e26c2ae478c261fa3fa2d84f52 Author: RafaelGSS <rafael.nunu@hotmail.com> Date: Mon Feb 13 15:41:30 2023 -0300 lib: makeRequireFunction patch when experimental policy Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/373 Refs: https://hackerone.com/bugs?subject=nodejs&report_id=1747642 CVE-ID: CVE-2023-23918 PR-URL: https://github.com/nodejs-private/node-private/pull/358 Reviewed-by: Bradley Farias <bradley.meck@gmail.com> Reviewed-by: Michael Dawson <midawson@redhat.com> commit 83975b7fb463e29c92c8e83693b725e269cee149 Author: RafaelGSS <rafael.nunu@hotmail.com> Date: Tue Oct 25 00:23:27 2022 -0300 policy: makeRequireFunction on mainModule.require Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> Co-authored-by: Bradley Farias <bradley.meck@gmail.com> Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/373 Refs: https://hackerone.com/bugs?subject=nodejs&report_id=1747642 CVE-ID: CVE-2023-23918 PR-URL: https://github.com/nodejs-private/node-private/pull/358 Reviewed-by: Bradley Farias <bradley.meck@gmail.com> Reviewed-by: Michael Dawson <midawson@redhat.com> commit fa115ee8ac32883c96df4a93fafb600bd665a5aa Author: Antoine du Hamel <duhamelantoine1995@gmail.com> Date: Fri Aug 5 00:41:48 2022 +0200 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> Index: node-v12.22.12/lib/internal/modules/cjs/helpers.js =================================================================== --- node-v12.22.12.orig/lib/internal/modules/cjs/helpers.js +++ node-v12.22.12/lib/internal/modules/cjs/helpers.js @@ -5,6 +5,7 @@ const { SafeMap, } = primordials; const { + ERR_INVALID_ARG_TYPE, ERR_MANIFEST_DEPENDENCY_MISSING, ERR_UNKNOWN_BUILTIN_MODULE } = require('internal/errors').codes; @@ -14,11 +15,22 @@ const { validateString } = require('inte const path = require('path'); const { pathToFileURL, fileURLToPath } = require('internal/url'); const { URL } = require('url'); +const { setOwnProperty } = require('internal/util'); + +const { + require_private_symbol, +} = internalBinding('util'); let debug = require('internal/util/debuglog').debuglog('module', (fn) => { debug = fn; }); +// TODO: Use this set when resolving pkg#exports conditions in loader.js. +const cjsConditions = [ + 'require', + 'node', +]; + function loadNativeModule(filename, request) { const mod = NativeModule.map.get(filename); if (mod) { @@ -28,20 +40,31 @@ function loadNativeModule(filename, requ } } +let $Module = null; +function lazyModule() { + $Module = $Module || require('internal/modules/cjs/loader').Module; + return $Module; +} + // Invoke with makeRequireFunction(module) where |module| is the Module object // to use as the context for the require() function. // Use redirects to set up a mapping from a policy and restrict dependencies const urlToFileCache = new SafeMap(); function makeRequireFunction(mod, redirects) { - const Module = mod.constructor; + // lazy due to cycle + const Module = lazyModule(); + if (mod instanceof Module !== true) { + throw new ERR_INVALID_ARG_TYPE('mod', 'Module', mod); + } let require; if (redirects) { const { resolve, reaction } = redirects; const id = mod.filename || mod.id; + const conditions = cjsConditions; require = function require(path) { let missing = true; - const destination = resolve(path); + const destination = resolve(path, conditions); if (destination === true) { missing = false; } else if (destination) { @@ -61,17 +84,17 @@ function makeRequireFunction(mod, redire filepath = fileURLToPath(destination); urlToFileCache.set(href, filepath); } - return mod.require(filepath); + return mod[require_private_symbol](mod, filepath); } } if (missing) { - reaction(new ERR_MANIFEST_DEPENDENCY_MISSING(id, path)); + reaction(new ERR_MANIFEST_DEPENDENCY_MISSING(id, path, ArrayPrototypeJoin([...conditions], ', '))); } - return mod.require(path); + return mod[require_private_symbol](mod, path); }; } else { require = function require(path) { - return mod.require(path); + return mod[require_private_symbol](mod, path); }; } @@ -89,7 +112,7 @@ function makeRequireFunction(mod, redire resolve.paths = paths; - require.main = process.mainModule; + setOwnProperty(require, 'main', process.mainModule); // Enable support to add extra extension types. require.extensions = Module._extensions; Index: node-v12.22.12/lib/internal/modules/cjs/loader.js =================================================================== --- node-v12.22.12.orig/lib/internal/modules/cjs/loader.js +++ node-v12.22.12/lib/internal/modules/cjs/loader.js @@ -59,7 +59,7 @@ const { rekeySourceMap } = 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'); @@ -70,6 +70,9 @@ const { internalModuleStat } = internalB const packageJsonReader = require('internal/modules/package_json_reader'); const { safeGetenv } = internalBinding('credentials'); const { + require_private_symbol, +} = internalBinding('util'); +const { makeRequireFunction, normalizeReferrerURL, stripBOM, @@ -120,6 +123,20 @@ const relativeResolveCache = ObjectCreat let requireDepth = 0; let statCache = null; +function internalRequire(module, id) { + validateString(id, 'id'); + if (id === '') { + throw new ERR_INVALID_ARG_VALUE('id', id, + 'must be a non-empty string'); + } + requireDepth++; + try { + return Module._load(id, module, /* isMain */ false); + } finally { + requireDepth--; + } +} + function stat(filename) { filename = path.toNamespacedPath(filename); if (statCache !== null) { @@ -140,12 +157,21 @@ function updateChildren(parent, child, s 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; this.loaded = false; this.children = []; + let redirects; + if (manifest) { + const moduleURL = pathToFileURL(id); + redirects = manifest.getRedirector(moduleURL); + } + setOwnProperty(this, 'require', makeRequireFunction(this, redirects)); + // Loads a module at the given file path. Returns that module's + // `exports` property. + this[require_private_symbol] = internalRequire; } const builtinModules = []; @@ -242,14 +268,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) { @@ -684,6 +709,7 @@ Module._load = function(request, parent, if (isMain) { process.mainModule = module; + setOwnProperty(module.require, 'main', process.mainModule); module.id = '.'; } @@ -873,24 +899,6 @@ Module.prototype.load = function(filenam ESMLoader.cjsCache.set(this, exports); }; - -// Loads a module at the given file path. Returns that module's -// `exports` property. -Module.prototype.require = function(id) { - validateString(id, 'id'); - if (id === '') { - throw new ERR_INVALID_ARG_VALUE('id', id, - 'must be a non-empty string'); - } - requireDepth++; - try { - return Module._load(id, this, /* isMain */ false); - } finally { - requireDepth--; - } -}; - - // Resolved path to process.argv[1] will be lazily placed here // (needed for setting breakpoint when called with --inspect-brk) let resolvedArgv; @@ -952,10 +960,9 @@ function wrapSafe(filename, content, cjs // Returns exception, if any. Module.prototype._compile = function(content, filename) { let moduleURL; - let redirects; if (manifest) { moduleURL = pathToFileURL(filename); - redirects = manifest.getRedirector(moduleURL); + manifest.getRedirector(moduleURL); manifest.assertIntegrity(moduleURL, content); } @@ -986,7 +993,6 @@ Module.prototype._compile = function(con } } const dirname = path.dirname(filename); - const require = makeRequireFunction(this, redirects); let result; const exports = this.exports; const thisValue = exports; @@ -994,9 +1000,9 @@ Module.prototype._compile = function(con if (requireDepth === 0) statCache = new Map(); if (inspectorWrapper) { result = inspectorWrapper(compiledWrapper, thisValue, exports, - require, module, filename, dirname); + module.require, module, filename, dirname); } else { - result = compiledWrapper.call(thisValue, exports, require, module, + result = compiledWrapper.call(thisValue, exports, module.require, module, filename, dirname); } hasLoadedAnyUserCJSModule = true; @@ -1038,7 +1044,7 @@ Module._extensions['.json'] = function(m } try { - module.exports = JSONParse(stripBOM(content)); + setOwnProperty(module, 'exports', JSONParse(stripBOM(content))); } catch (err) { err.message = filename + ': ' + err.message; throw err; @@ -1144,7 +1150,7 @@ Module._preloadModules = function(reques } } for (let n = 0; n < requests.length; n++) - parent.require(requests[n]); + internalRequire(parent, requests[n]); }; Module.syncBuiltinESMExports = function syncBuiltinESMExports() { @@ -1155,5 +1161,14 @@ Module.syncBuiltinESMExports = function } }; +ObjectDefineProperty(Module.prototype, 'constructor', { + __proto__: null, + get: function() { + return policy ? undefined : Module; + }, + configurable: false, + enumerable: false, +}); + // Backwards compatibility Module.Module = Module; Index: node-v12.22.12/lib/internal/util.js =================================================================== --- node-v12.22.12.orig/lib/internal/util.js +++ node-v12.22.12/lib/internal/util.js @@ -11,6 +11,7 @@ const { ObjectGetOwnPropertyDescriptor, ObjectGetOwnPropertyDescriptors, ObjectGetPrototypeOf, + ObjectPrototypeHasOwnProperty, ObjectSetPrototypeOf, Promise, ReflectConstruct, @@ -404,6 +405,35 @@ function sleep(msec) { _sleep(msec); } +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, @@ -413,6 +443,7 @@ module.exports = { deprecate, emitExperimentalWarning, filterDuplicateStrings, + filterOwnProperties, getConstructorOf, getSystemErrorName, isError, @@ -435,5 +466,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, }; Index: node-v12.22.12/test/common/fixtures.js =================================================================== --- node-v12.22.12.orig/test/common/fixtures.js +++ node-v12.22.12/test/common/fixtures.js @@ -3,6 +3,7 @@ const path = require('path'); const fs = require('fs'); +const { pathToFileURL } = require('url'); const fixturesDir = path.join(__dirname, '..', 'fixtures'); @@ -10,6 +11,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); @@ -23,6 +28,7 @@ function readFixtureKey(name, enc) { module.exports = { fixturesDir, path: fixturesPath, + fileURL: fixturesFileURL, readSync: readFixtureSync, readKey: readFixtureKey }; Index: node-v12.22.12/test/fixtures/es-module-specifiers/index.mjs =================================================================== --- node-v12.22.12.orig/test/fixtures/es-module-specifiers/index.mjs +++ node-v12.22.12/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'; Index: node-v12.22.12/test/fixtures/es-module-specifiers/node_modules/no-main-field/index.js =================================================================== --- /dev/null +++ node-v12.22.12/test/fixtures/es-module-specifiers/node_modules/no-main-field/index.js @@ -0,0 +1,3 @@ +'use strict'; +module.exports = 'no main field'; + Index: node-v12.22.12/test/fixtures/es-module-specifiers/node_modules/no-main-field/package.json =================================================================== --- /dev/null +++ node-v12.22.12/test/fixtures/es-module-specifiers/node_modules/no-main-field/package.json @@ -0,0 +1 @@ +{} Index: node-v12.22.12/test/parallel/test-module-prototype-mutation.js =================================================================== --- /dev/null +++ node-v12.22.12/test/parallel/test-module-prototype-mutation.js @@ -0,0 +1,13 @@ +'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'))); + Index: node-v12.22.12/test/message/source_map_throw_catch.out =================================================================== --- node-v12.22.12.orig/test/message/source_map_throw_catch.out +++ node-v12.22.12/test/message/source_map_throw_catch.out @@ -8,7 +8,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:*) Index: node-v12.22.12/test/message/source_map_throw_first_tick.out =================================================================== --- node-v12.22.12.orig/test/message/source_map_throw_first_tick.out +++ node-v12.22.12/test/message/source_map_throw_first_tick.out @@ -8,7 +8,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:*) Index: node-v12.22.12/src/env.h =================================================================== --- node-v12.22.12.orig/src/env.h +++ node-v12.22.12/src/env.h @@ -161,6 +161,7 @@ constexpr size_t kFsStatsBufferLength = V(napi_wrapper, "node:napi:wrapper") \ V(sab_lifetimepartner_symbol, "node:sharedArrayBufferLifetimePartner") \ V(untransferable_object_private_symbol, "node:untransferableObject") \ + V(require_private_symbol, "node:require_private_symbol") // Symbols are per-isolate primitives but Environment proxies them // for the sake of convenience. Index: node-v12.22.12/test/fixtures/policy-manifest/object-define-property-bypass.js =================================================================== --- /dev/null +++ node-v12.22.12/test/fixtures/policy-manifest/object-define-property-bypass.js @@ -0,0 +1,19 @@ +let requires = new WeakMap() +Object.defineProperty(Object.getPrototypeOf(module), 'require', { + get() { + return requires.get(this); + }, + set(v) { + requires.set(this, v); + process.nextTick(() => { + let fs = Reflect.apply(v, this, ['fs']) + if (typeof fs.readFileSync === 'function') { + process.exit(1); + } + }) + return requires.get(this); + }, + configurable: true +}) + +require('./valid-module') Index: node-v12.22.12/test/fixtures/policy-manifest/onerror-exit.json =================================================================== --- /dev/null +++ node-v12.22.12/test/fixtures/policy-manifest/onerror-exit.json @@ -0,0 +1,9 @@ +{ + "onerror": "exit", + "scopes": { + "file:": { + "integrity": true, + "dependencies": {} + } + } +} Index: node-v12.22.12/test/fixtures/policy-manifest/onerror-resource-exit.json =================================================================== --- /dev/null +++ node-v12.22.12/test/fixtures/policy-manifest/onerror-resource-exit.json @@ -0,0 +1,17 @@ +{ + "onerror": "exit", + "resources": { + "./object-define-property-bypass.js": { + "integrity": true, + "dependencies": { + "./valid-module": true + } + }, + "./valid-module.js": { + "integrity": true, + "dependencies": { + "fs": true + } + } + } +} Index: node-v12.22.12/test/parallel/test-policy-manifest.js =================================================================== --- /dev/null +++ node-v12.22.12/test/parallel/test-policy-manifest.js @@ -0,0 +1,63 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +common.requireNoPackageJSONAbove(); + +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const fixtures = require('../common/fixtures.js'); + +{ + const policyFilepath = fixtures.path('policy-manifest', 'onerror-exit.json'); + const result = spawnSync(process.execPath, [ + '--experimental-policy', + policyFilepath, + '-e', + 'require("os").cpus()', + ]); + + assert.notStrictEqual(result.status, 0); + const stderr = result.stderr.toString(); + assert.match(stderr, /ERR_MANIFEST_ASSERT_INTEGRITY/); + assert.match(stderr, /The resource was not found in the policy/); +} + +{ + const policyFilepath = fixtures.path('policy-manifest', 'onerror-exit.json'); + const mainModuleBypass = fixtures.path( + 'policy-manifest', + 'main-module-bypass.js', + ); + const result = spawnSync(process.execPath, [ + '--experimental-policy', + policyFilepath, + mainModuleBypass, + ]); + + assert.notStrictEqual(result.status, 0); + const stderr = result.stderr.toString(); + assert.match(stderr, /ERR_MANIFEST_ASSERT_INTEGRITY/); + assert.match(stderr, /The resource was not found in the policy/); +} + +{ + const policyFilepath = fixtures.path( + 'policy-manifest', + 'onerror-resource-exit.json', + ); + const objectDefinePropertyBypass = fixtures.path( + 'policy-manifest', + 'object-define-property-bypass.js', + ); + const result = spawnSync(process.execPath, [ + '--experimental-policy', + policyFilepath, + objectDefinePropertyBypass, + ]); + + assert.strictEqual(result.status, 0); +} Index: node-v12.22.12/test/common/index.js =================================================================== --- node-v12.22.12.orig/test/common/index.js +++ node-v12.22.12/test/common/index.js @@ -710,6 +710,20 @@ function gcUntil(name, condition) { }); } +function requireNoPackageJSONAbove(dir = __dirname) { + let possiblePackage = path.join(dir, '..', 'package.json'); + let lastPackage = null; + while (possiblePackage !== lastPackage) { + if (fs.existsSync(possiblePackage)) { + assert.fail( + 'This test shouldn\'t load properties from a package.json above ' + + `its file location. Found package.json at ${possiblePackage}.`); + } + lastPackage = possiblePackage; + possiblePackage = path.join(possiblePackage, '..', '..', 'package.json'); + } +} + const common = { allowGlobals, buildType, @@ -751,6 +765,7 @@ const common = { platformTimeout, printSkipMessage, pwdCommand, + requireNoPackageJSONAbove, runWithInvalidFD, skip, skipIf32Bits, Index: node-v12.22.12/lib/internal/policy/manifest.js =================================================================== --- node-v12.22.12.orig/lib/internal/policy/manifest.js +++ node-v12.22.12/lib/internal/policy/manifest.js @@ -115,7 +115,7 @@ class Manifest { } this.#reaction = reaction; - const manifestEntries = ObjectEntries(obj.resources); + const manifestEntries = ObjectEntries(obj.resources ? obj.resources : ObjectCreate(null)); const parsedURLs = new SafeMap(); for (let i = 0; i < manifestEntries.length; i++) { Index: node-v12.22.12/test/fixtures/policy-manifest/valid-module.js =================================================================== --- /dev/null +++ node-v12.22.12/test/fixtures/policy-manifest/valid-module.js @@ -0,0 +1,2 @@ + + Index: node-v12.22.12/test/fixtures/policy-manifest/main-module-bypass.js =================================================================== --- /dev/null +++ node-v12.22.12/test/fixtures/policy-manifest/main-module-bypass.js @@ -0,0 +1 @@ +process.mainModule.require('os').cpus();
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
