Skip to content

Commit 5cbe7c2

Browse files
addaleaxtargos
authored andcommitted
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()`. PR-URL: #30228 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 338d216 commit 5cbe7c2

2 files changed

Lines changed: 91 additions & 12 deletions

File tree

‎lib/internal/source_map/source_map_cache.js‎

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,28 @@
11
'use strict';
22

3+
const{
4+
JSON,
5+
Object: {
6+
create: ObjectCreate,
7+
keys: ObjectKeys,
8+
getOwnPropertyDescriptor: ObjectGetOwnPropertyDescriptor,
9+
},
10+
ObjectPrototype: {
11+
hasOwnProperty: ObjectHasOwnProperty
12+
},
13+
MapPrototype: {
14+
entries: MapEntries
15+
}, uncurryThis
16+
}=primordials;
17+
18+
constMapIteratorNext=uncurryThis(MapEntries(newMap()).next);
19+
constWeakMapGet=uncurryThis(WeakMap.prototype.get);
20+
21+
functionObjectGetValueSafe(obj,key){
22+
constdesc=ObjectGetOwnPropertyDescriptor(obj,key);
23+
returnObjectHasOwnProperty(desc,'value') ? desc.value : undefined;
24+
}
25+
326
// See https://sourcemaps.info/spec.html for SourceMap V3 specification.
427
const{ Buffer }=require('buffer');
528
constdebug=require('internal/util/debuglog').debuglog('source_map');
@@ -9,14 +32,14 @@ const { getOptionValue } = require('internal/options');
932
const{
1033
normalizeReferrerURL,
1134
}=require('internal/modules/cjs/helpers');
12-
const{JSON, Object }=primordials;
1335
// For cjs, since Module._cache is exposed to users, we use a WeakMap
1436
// keyed on module, facilitating garbage collection.
1537
constcjsSourceMapCache=newWeakMap();
1638
// The esm cache is not exposed to users, so we can use a Map keyed
1739
// on filenames.
1840
constesmSourceMapCache=newMap();
1941
const{ fileURLToPath,URL}=require('url');
42+
letModule;
2043

2144
letexperimentalSourceMaps;
2245
functionmaybeCacheSourceMap(filename,content,cjsModuleInstance){
@@ -40,6 +63,7 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
4063
constdata=dataFromUrl(basePath,match.groups.sourceMappingURL);
4164
consturl=data ? null : match.groups.sourceMappingURL;
4265
if(cjsModuleInstance){
66+
if(!Module)Module=require('internal/modules/cjs/loader').Module;
4367
cjsSourceMapCache.set(cjsModuleInstance,{
4468
filename,
4569
lineLengths: lineLengths(content),
@@ -148,17 +172,27 @@ function rekeySourceMap(cjsModuleInstance, newInstance) {
148172
}
149173
}
150174

175+
// WARNING: The `sourceMapCacheToObject` and `appendCJSCache` run during
176+
// shutdown. In particular, they also run when Workers are terminated, making
177+
// it important that they do not call out to any user-provided code, including
178+
// built-in prototypes that might have been tampered with.
179+
151180
// Get serialized representation of source-map cache, this is used
152181
// to persist a cache of source-maps to disk when NODE_V8_COVERAGE is enabled.
153182
functionsourceMapCacheToObject(){
154-
constobj=Object.create(null);
183+
constobj=ObjectCreate(null);
155184

156-
for(const[k,v]ofesmSourceMapCache){
185+
constit=MapEntries(esmSourceMapCache);
186+
letentry;
187+
while(!(entry=MapIteratorNext(it)).done){
188+
constk=entry.value[0];
189+
constv=entry.value[1];
157190
obj[k]=v;
158191
}
192+
159193
appendCJSCache(obj);
160194

161-
if(Object.keys(obj).length===0){
195+
if(ObjectKeys(obj).length===0){
162196
returnundefined;
163197
}else{
164198
returnobj;
@@ -171,23 +205,28 @@ function sourceMapCacheToObject() {
171205
// TODO(bcoe): this means we don't currently serialize source-maps attached
172206
// to error instances, only module instances.
173207
functionappendCJSCache(obj){
174-
const{ Module }=require('internal/modules/cjs/loader');
175-
Object.keys(Module._cache).forEach((key)=>{
176-
constvalue=cjsSourceMapCache.get(Module._cache[key]);
208+
if(!Module)return;
209+
constcjsModuleCache=ObjectGetValueSafe(Module,'_cache');
210+
constcjsModules=ObjectKeys(cjsModuleCache);
211+
for(leti=0;i<cjsModules.length;i++){
212+
constkey=cjsModules[i];
213+
constmodule=ObjectGetValueSafe(cjsModuleCache,key);
214+
constvalue=WeakMapGet(cjsSourceMapCache,module);
177215
if(value){
216+
// This is okay because `obj` has a null prototype.
178217
obj[`file://${key}`]={
179-
lineLengths: value.lineLengths,
180-
data: value.data,
181-
url: value.url
218+
lineLengths: ObjectGetValueSafe(value,'lineLengths'),
219+
data: ObjectGetValueSafe(value,'data'),
220+
url: ObjectGetValueSafe(value,'url')
182221
};
183222
}
184-
});
223+
}
185224
}
186225

187226
// Attempt to lookup a source map, which is either attached to a file URI, or
188227
// keyed on an error instance.
189228
functionfindSourceMap(uri,error){
190-
const{ Module }=require('internal/modules/cjs/loader');
229+
if(!Module)Module=require('internal/modules/cjs/loader').Module;
191230
letsourceMap=cjsSourceMapCache.get(Module._cache[uri]);
192231
if(!uri.startsWith('file://'))uri=normalizeReferrerURL(uri);
193232
if(sourceMap===undefined){
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
'use strict';
2+
constcommon=require('../common');
3+
consttmpdir=require('../common/tmpdir');
4+
constassert=require('assert');
5+
6+
// Attempts to test that the source map JS code run on process shutdown
7+
// does not call any user-defined JS code.
8+
9+
const{ Worker, workerData, parentPort }=require('worker_threads');
10+
11+
if(!workerData){
12+
tmpdir.refresh();
13+
process.env.NODE_V8_COVERAGE=tmpdir.path;
14+
15+
// Count the number of some calls that should not be made.
16+
constcallCount=newInt32Array(newSharedArrayBuffer(4));
17+
constw=newWorker(__filename,{workerData: { callCount }});
18+
w.on('message',common.mustCall(()=>w.terminate()));
19+
w.on('exit',common.mustCall(()=>{
20+
assert.strictEqual(callCount[0],0);
21+
}));
22+
return;
23+
}
24+
25+
const{ callCount }=workerData;
26+
27+
functionincreaseCallCount(){callCount[0]++;}
28+
29+
// Increase the call count when a forbidden method is called.
30+
Object.getPrototypeOf((newMap()).entries()).next=increaseCallCount;
31+
Map.prototype.entries=increaseCallCount;
32+
Object.keys=increaseCallCount;
33+
Object.create=increaseCallCount;
34+
Object.hasOwnProperty=increaseCallCount;
35+
Object.defineProperty(Object.prototype,'value',{
36+
get: increaseCallCount,
37+
set: increaseCallCount
38+
});
39+
40+
parentPort.postMessage('done');

0 commit comments

Comments
 (0)