File CVE-2023-23918.patch of Package nodejs12.30292
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();
