Skip to content

Commit 46ded6b

Browse files
aduh95danielleadams
authored andcommitted
esm: protect ESM loader from prototype pollution
Fixes: #45035 PR-URL: #45044 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent f222c95 commit 46ded6b

File tree

8 files changed

+67
-6
lines changed

8 files changed

+67
-6
lines changed

lib/internal/bootstrap/loaders.js

+3
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ const {
5353
ObjectDefineProperty,
5454
ObjectKeys,
5555
ObjectPrototypeHasOwnProperty,
56+
ObjectSetPrototypeOf,
5657
ReflectGet,
5758
SafeMap,
5859
SafeSet,
@@ -281,6 +282,8 @@ class BuiltinModule {
281282
getESMFacade() {
282283
if (this.module) return this.module;
283284
const { ModuleWrap } = internalBinding('module_wrap');
285+
// TODO(aduh95): move this to C++, alongside the initialization of the class.
286+
ObjectSetPrototypeOf(ModuleWrap.prototype, null);
284287
const url = `node:${this.id}`;
285288
const nativeModule = this;
286289
const exportsKeys = ArrayPrototypeSlice(this.exportKeys);

lib/internal/modules/esm/load.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ async function getSource(url, context) {
5959
if (policy?.manifest) {
6060
policy.manifest.assertIntegrity(parsed, source);
6161
}
62-
return { responseURL, source };
62+
return { __proto__: null, responseURL, source };
6363
}
6464

6565

@@ -93,6 +93,7 @@ async function defaultLoad(url, context) {
9393
}
9494

9595
return {
96+
__proto__: null,
9697
format,
9798
responseURL,
9899
source,

lib/internal/modules/esm/loader.js

+2
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,7 @@ class ESMLoader {
676676
}
677677

678678
return {
679+
__proto__: null,
679680
format,
680681
responseURL,
681682
source,
@@ -892,6 +893,7 @@ class ESMLoader {
892893
}
893894

894895
return {
896+
__proto__: null,
895897
format,
896898
url,
897899
};

lib/internal/modules/esm/module_job.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ class ModuleJob {
215215
}
216216
throw e;
217217
}
218-
return { module: this.module };
218+
return { __proto__: null, module: this.module };
219219
}
220220
}
221221
ObjectSetPrototypeOf(ModuleJob.prototype, null);

lib/internal/modules/esm/resolve.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1083,7 +1083,7 @@ async function defaultResolve(specifier, context = {}) {
10831083
)
10841084
)
10851085
) {
1086-
return { url: parsed.href };
1086+
return { __proto__: null, url: parsed.href };
10871087
}
10881088
} catch {
10891089
// Ignore exception
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
'use strict';
2+
3+
const { mustNotCall, mustCall } = require('../common');
4+
5+
Object.defineProperties(Array.prototype, {
6+
// %Promise.all% and %Promise.allSettled% are depending on the value of
7+
// `%Array.prototype%.then`.
8+
then: {},
9+
});
10+
Object.defineProperties(Object.prototype, {
11+
then: {
12+
set: mustNotCall('set %Object.prototype%.then'),
13+
get: mustNotCall('get %Object.prototype%.then'),
14+
},
15+
});
16+
17+
import('data:text/javascript,').then(mustCall());
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { mustNotCall, mustCall } from '../common/index.mjs';
2+
3+
Object.defineProperties(Array.prototype, {
4+
// %Promise.all% and %Promise.allSettled% are depending on the value of
5+
// `%Array.prototype%.then`.
6+
then: {},
7+
});
8+
Object.defineProperties(Object.prototype, {
9+
then: {
10+
set: mustNotCall('set %Object.prototype%.then'),
11+
get: mustNotCall('get %Object.prototype%.then'),
12+
},
13+
});
14+
15+
import('data:text/javascript,').then(mustCall());

test/parallel/test-primordials-promise.js

+26-3
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,32 @@ Promise.all = common.mustNotCall('%Promise%.all');
1818
Promise.allSettled = common.mustNotCall('%Promise%.allSettled');
1919
Promise.any = common.mustNotCall('%Promise%.any');
2020
Promise.race = common.mustNotCall('%Promise%.race');
21-
Promise.prototype.catch = common.mustNotCall('%Promise.prototype%.catch');
22-
Promise.prototype.finally = common.mustNotCall('%Promise.prototype%.finally');
23-
Promise.prototype.then = common.mustNotCall('%Promise.prototype%.then');
21+
22+
Object.defineProperties(Promise.prototype, {
23+
catch: {
24+
set: common.mustNotCall('set %Promise.prototype%.catch'),
25+
get: common.mustNotCall('get %Promise.prototype%.catch'),
26+
},
27+
finally: {
28+
set: common.mustNotCall('set %Promise.prototype%.finally'),
29+
get: common.mustNotCall('get %Promise.prototype%.finally'),
30+
},
31+
then: {
32+
set: common.mustNotCall('set %Promise.prototype%.then'),
33+
get: common.mustNotCall('get %Promise.prototype%.then'),
34+
},
35+
});
36+
Object.defineProperties(Array.prototype, {
37+
// %Promise.all% and %Promise.allSettled% are depending on the value of
38+
// `%Array.prototype%.then`.
39+
then: {},
40+
});
41+
Object.defineProperties(Object.prototype, {
42+
then: {
43+
set: common.mustNotCall('set %Object.prototype%.then'),
44+
get: common.mustNotCall('get %Object.prototype%.then'),
45+
},
46+
});
2447

2548
assertIsPromise(PromisePrototypeThen(test(), common.mustCall()));
2649
assertIsPromise(SafePromisePrototypeFinally(test(), common.mustCall()));

0 commit comments

Comments
 (0)