Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 46ded6b

Browse filesBrowse files
aduh95danielleadams
aduh95
authored and
danielleadams
committed
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
Copy full SHA for 46ded6b

File tree

Expand file treeCollapse file tree

8 files changed

+67
-6
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+67
-6
lines changed
Open diff view settings
Collapse file

‎lib/internal/bootstrap/loaders.js‎

Copy file name to clipboardExpand all lines: lib/internal/bootstrap/loaders.js
+3Lines changed: 3 additions & 0 deletions
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);
Collapse file

‎lib/internal/modules/esm/load.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/load.js
+2-1Lines changed: 2 additions & 1 deletion
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,
Collapse file

‎lib/internal/modules/esm/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/loader.js
+2Lines changed: 2 additions & 0 deletions
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
};
Collapse file

‎lib/internal/modules/esm/module_job.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/module_job.js
+1-1Lines changed: 1 addition & 1 deletion
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);
Collapse file

‎lib/internal/modules/esm/resolve.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/resolve.js
+1-1Lines changed: 1 addition & 1 deletion
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
Collapse file

‎test/es-module/test-cjs-prototype-pollution.js‎

Copy file name to clipboard
+17Lines changed: 17 additions & 0 deletions
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());
Collapse file

‎test/es-module/test-esm-prototype-pollution.mjs‎

Copy file name to clipboard
+15Lines changed: 15 additions & 0 deletions
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());
Collapse file

‎test/parallel/test-primordials-promise.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-primordials-promise.js
+26-3Lines changed: 26 additions & 3 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.