From ba02c7421d4da7f837d7e23352ece10f99c5557f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 2 Nov 2019 18:33:38 +0100 Subject: [PATCH 1/3] process: make source map getter resistant against prototype tampering Since this code runs during process and Worker shutdown, it should not call user-provided code and thereby e.g. provide a way to break out of `worker.terminate()`. --- lib/internal/source_map/source_map_cache.js | 53 ++++++++++++++----- .../test-worker-terminate-source-map.js | 40 ++++++++++++++ 2 files changed, 81 insertions(+), 12 deletions(-) create mode 100644 test/parallel/test-worker-terminate-source-map.js diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index 0e77203e9d6f1d..4c73bc5389df5f 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -9,7 +9,7 @@ const { getOptionValue } = require('internal/options'); const { normalizeReferrerURL, } = require('internal/modules/cjs/helpers'); -const { JSON, Object } = primordials; +const { JSON, Object, uncurryThis } = primordials; // For cjs, since Module._cache is exposed to users, we use a WeakMap // keyed on module, facilitating garbage collection. const cjsSourceMapCache = new WeakMap(); @@ -17,6 +17,7 @@ const cjsSourceMapCache = new WeakMap(); // on filenames. const esmSourceMapCache = new Map(); const { fileURLToPath, URL } = require('url'); +let Module; let experimentalSourceMaps; function maybeCacheSourceMap(filename, content, cjsModuleInstance) { @@ -40,6 +41,7 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) { const data = dataFromUrl(basePath, match.groups.sourceMappingURL); const url = data ? null : match.groups.sourceMappingURL; if (cjsModuleInstance) { + if (!Module) Module = require('internal/modules/cjs/loader').Module; cjsSourceMapCache.set(cjsModuleInstance, { filename, lineLengths: lineLengths(content), @@ -148,17 +150,39 @@ function rekeySourceMap(cjsModuleInstance, newInstance) { } } +// WARNING: The `sourceMapCacheToObject` and `appendCJSCache` run during +// shutdown. In particular, they also run when Workers are terminated, making +// it important that they do not call out to any user-provided code, including +// built-in prototypes that might have been tampered with. +const ObjectCreate = Object.create; +const ObjectKeys = Object.keys; +const ObjectGetOwnPropertyDescriptor = Object.getOwnPropertyDescriptor; +const ObjectHasOwnProperty = uncurryThis(Object.prototype.hasOwnProperty); +const MapEntries = uncurryThis(Map.prototype.entries); +const MapIteratorNext = uncurryThis(MapEntries(new Map()).next); +const WeakMapGet = uncurryThis(WeakMap.prototype.get); + +function ObjectGetValueSafe(obj, key) { + const desc = ObjectGetOwnPropertyDescriptor(obj, key); + return ObjectHasOwnProperty(desc, 'value') ? desc.value : undefined; +} + // Get serialized representation of source-map cache, this is used // to persist a cache of source-maps to disk when NODE_V8_COVERAGE is enabled. function sourceMapCacheToObject() { - const obj = Object.create(null); + const obj = ObjectCreate(null); - for (const [k, v] of esmSourceMapCache) { + const it = MapEntries(esmSourceMapCache); + let entry; + while (!(entry = MapIteratorNext(it)).done) { + const k = entry.value[0]; + const v = entry.value[1]; obj[k] = v; } + appendCJSCache(obj); - if (Object.keys(obj).length === 0) { + if (ObjectKeys(obj).length === 0) { return undefined; } else { return obj; @@ -171,23 +195,28 @@ function sourceMapCacheToObject() { // TODO(bcoe): this means we don't currently serialize source-maps attached // to error instances, only module instances. function appendCJSCache(obj) { - const { Module } = require('internal/modules/cjs/loader'); - Object.keys(Module._cache).forEach((key) => { - const value = cjsSourceMapCache.get(Module._cache[key]); + if (!Module) return; + const cjsModuleCache = ObjectGetValueSafe(Module, '_cache'); + const cjsModules = ObjectKeys(cjsModuleCache); + for (let i = 0; i < cjsModules.length; i++) { + const key = cjsModules[i]; + const module = ObjectGetValueSafe(cjsModuleCache, key); + const value = WeakMapGet(cjsSourceMapCache, module); if (value) { + // This is okay because `obj` has a null prototype. obj[`file://${key}`] = { - lineLengths: value.lineLengths, - data: value.data, - url: value.url + lineLengths: ObjectGetValueSafe(value, 'lineLengths'), + data: ObjectGetValueSafe(value, 'data'), + url: ObjectGetValueSafe(value, 'url') }; } - }); + } } // Attempt to lookup a source map, which is either attached to a file URI, or // keyed on an error instance. function findSourceMap(uri, error) { - const { Module } = require('internal/modules/cjs/loader'); + if (!Module) Module = require('internal/modules/cjs/loader').Module; let sourceMap = cjsSourceMapCache.get(Module._cache[uri]); if (!uri.startsWith('file://')) uri = normalizeReferrerURL(uri); if (sourceMap === undefined) { diff --git a/test/parallel/test-worker-terminate-source-map.js b/test/parallel/test-worker-terminate-source-map.js new file mode 100644 index 00000000000000..d88a4c68366865 --- /dev/null +++ b/test/parallel/test-worker-terminate-source-map.js @@ -0,0 +1,40 @@ +'use strict'; +const common = require('../common'); +const tmpdir = require('../common/tmpdir'); +const assert = require('assert'); + +// Attempts to test that the source map JS code run on process shutdown +// does not call any user-defined JS code. + +const { Worker, workerData, parentPort } = require('worker_threads'); + +if (!workerData) { + tmpdir.refresh(); + process.env.NODE_V8_COVERAGE = tmpdir.path; + + // Count the number of some calls that should not be made. + const callCount = new Int32Array(new SharedArrayBuffer(4)); + const w = new Worker(__filename, { workerData: { callCount } }); + w.on('message', common.mustCall(() => w.terminate())); + w.on('exit', common.mustCall(() => { + assert.strictEqual(callCount[0], 0); + })); + return; +} + +const { callCount } = workerData; + +function increaseCallCount() { callCount[0]++; } + +// Increase the call count when a forbidden method is called. +Object.getPrototypeOf((new Map()).entries()).next = increaseCallCount; +Map.prototype.entries = increaseCallCount; +Object.keys = increaseCallCount; +Object.create = increaseCallCount; +Object.hasOwnProperty = increaseCallCount; +Object.defineProperty(Object.prototype, 'value', { + get: increaseCallCount, + set: increaseCallCount +}); + +parentPort.postMessage('done'); From 2a121a0d512914c1a86bb0c301341f2153c59618 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 2 Nov 2019 22:21:28 +0100 Subject: [PATCH 2/3] fixup! process: make source map getter resistant against prototype tampering --- lib/internal/source_map/source_map_cache.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index 4c73bc5389df5f..9fe2aa67e599ca 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -9,7 +9,9 @@ const { getOptionValue } = require('internal/options'); const { normalizeReferrerURL, } = require('internal/modules/cjs/helpers'); -const { JSON, Object, uncurryThis } = primordials; +const { + JSON, Object, ObjectPrototype, MapPrototype, uncurryThis +} = primordials; // For cjs, since Module._cache is exposed to users, we use a WeakMap // keyed on module, facilitating garbage collection. const cjsSourceMapCache = new WeakMap(); @@ -157,8 +159,8 @@ function rekeySourceMap(cjsModuleInstance, newInstance) { const ObjectCreate = Object.create; const ObjectKeys = Object.keys; const ObjectGetOwnPropertyDescriptor = Object.getOwnPropertyDescriptor; -const ObjectHasOwnProperty = uncurryThis(Object.prototype.hasOwnProperty); -const MapEntries = uncurryThis(Map.prototype.entries); +const ObjectHasOwnProperty = ObjectPrototype.hasOwnProperty; +const MapEntries = MapPrototype.entries; const MapIteratorNext = uncurryThis(MapEntries(new Map()).next); const WeakMapGet = uncurryThis(WeakMap.prototype.get); From 24905db9b71cdbd643829e226cd4f2b69f6fa3b7 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 4 Nov 2019 08:45:20 +0100 Subject: [PATCH 3/3] fixup! process: make source map getter resistant against prototype tampering --- lib/internal/source_map/source_map_cache.js | 38 +++++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index 9fe2aa67e599ca..340615eb6cffcd 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -1,5 +1,28 @@ 'use strict'; +const { + JSON, + Object: { + create: ObjectCreate, + keys: ObjectKeys, + getOwnPropertyDescriptor: ObjectGetOwnPropertyDescriptor, + }, + ObjectPrototype: { + hasOwnProperty: ObjectHasOwnProperty + }, + MapPrototype: { + entries: MapEntries + }, uncurryThis +} = primordials; + +const MapIteratorNext = uncurryThis(MapEntries(new Map()).next); +const WeakMapGet = uncurryThis(WeakMap.prototype.get); + +function ObjectGetValueSafe(obj, key) { + const desc = ObjectGetOwnPropertyDescriptor(obj, key); + return ObjectHasOwnProperty(desc, 'value') ? desc.value : undefined; +} + // See https://sourcemaps.info/spec.html for SourceMap V3 specification. const { Buffer } = require('buffer'); const debug = require('internal/util/debuglog').debuglog('source_map'); @@ -9,9 +32,6 @@ const { getOptionValue } = require('internal/options'); const { normalizeReferrerURL, } = require('internal/modules/cjs/helpers'); -const { - JSON, Object, ObjectPrototype, MapPrototype, uncurryThis -} = primordials; // For cjs, since Module._cache is exposed to users, we use a WeakMap // keyed on module, facilitating garbage collection. const cjsSourceMapCache = new WeakMap(); @@ -156,18 +176,6 @@ function rekeySourceMap(cjsModuleInstance, newInstance) { // shutdown. In particular, they also run when Workers are terminated, making // it important that they do not call out to any user-provided code, including // built-in prototypes that might have been tampered with. -const ObjectCreate = Object.create; -const ObjectKeys = Object.keys; -const ObjectGetOwnPropertyDescriptor = Object.getOwnPropertyDescriptor; -const ObjectHasOwnProperty = ObjectPrototype.hasOwnProperty; -const MapEntries = MapPrototype.entries; -const MapIteratorNext = uncurryThis(MapEntries(new Map()).next); -const WeakMapGet = uncurryThis(WeakMap.prototype.get); - -function ObjectGetValueSafe(obj, key) { - const desc = ObjectGetOwnPropertyDescriptor(obj, key); - return ObjectHasOwnProperty(desc, 'value') ? desc.value : undefined; -} // Get serialized representation of source-map cache, this is used // to persist a cache of source-maps to disk when NODE_V8_COVERAGE is enabled.