FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

tools,doc: add guards against prototype pollution when creating proxies · nodejs/node@812140c · GitHub

/ node Public

Commit 812140c

Browse files
aduh95
authored and
danielleadams
committed
tools,doc: add guards against prototype pollution when creating proxies
PR-URL: #43391 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
1 parent ac9599a commit 812140c

6 files changed

Lines changed: 67 additions & 0 deletions

File tree

‎doc/contributing/primordials.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,3 +705,27 @@ class SomeClass {
705705
ObjectDefineProperty(SomeClass.prototype, 'readOnlyProperty', kEnumerableProperty);
706706
console.log(new SomeClass().readOnlyProperty); // genuine data
707707
```
708+
709+
### Defining a `Proxy` handler
710+
711+
When defining a `Proxy`, the handler object could be at risk of prototype
712+
pollution when using a plain object literal:
713+
714+
```js
715+
// User-land
716+
Object.prototype.get = () => 'Unrelated user-provided data';
717+
718+
// Core
719+
const objectToProxy = { someProperty: 'genuine value' };
720+
721+
const proxyWithPlainObjectLiteral = new Proxy(objectToProxy, {
722+
has() { return false; },
723+
});
724+
console.log(proxyWithPlainObjectLiteral.someProperty); // Unrelated user-provided data
725+
726+
const proxyWithNullPrototypeObject = new Proxy(objectToProxy, {
727+
__proto__: null,
728+
has() { return false; },
729+
});
730+
console.log(proxyWithNullPrototypeObject.someProperty); // genuine value
731+
```

‎lib/internal/debugger/inspect.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ function createAgentProxy(domain, client) {
117117
};
118118

119119
return new Proxy(agent, {
120+
__proto__: null,
120121
get(target, name) {
121122
if (name in target) return target[name];
122123
return function callVirtualMethod(params) {

‎lib/internal/http2/core.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -987,6 +987,7 @@ function trackAssignmentsTypedArray(typedArray) {
987987
}
988988

989989
return new Proxy(typedArray, {
990+
__proto__: null,
990991
get(obj, prop, receiver) {
991992
if (prop === 'copyAssigned') {
992993
return copyAssigned;

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,8 @@ const wrapper = [
210210
];
211211

212212
let wrapperProxy = new Proxy(wrapper, {
213+
__proto__: null,
214+
213215
set(target, property, value, receiver) {
214216
patched = true;
215217
return ReflectSet(target, property, value, receiver);
@@ -718,6 +720,8 @@ function emitCircularRequireWarning(prop) {
718720
// A Proxy that can be used as the prototype of a module.exports object and
719721
// warns when non-existent properties are accessed.
720722
const CircularRequirePrototypeWarningProxy = new Proxy({}, {
723+
__proto__: null,
724+
721725
get(target, prop) {
722726
// Allow __esModule access in any case because it is used in the output
723727
// of transpiled code to determine whether something comes from an

‎test/parallel/test-eslint-avoid-prototype-pollution.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ new RuleTester({
4545
'ReflectDefineProperty({}, "key", { "__proto__": null })',
4646
'ObjectDefineProperty({}, "key", { \'__proto__\': null })',
4747
'ReflectDefineProperty({}, "key", { \'__proto__\': null })',
48+
'new Proxy({}, otherObject)',
49+
'new Proxy({}, someFactory())',
50+
'new Proxy({}, { __proto__: null })',
51+
'new Proxy({}, { __proto__: null, ...{} })',
4852
],
4953
invalid: [
5054
{
@@ -183,5 +187,21 @@ new RuleTester({
183187
code: 'StringPrototypeSplit("some string", /some regex/)',
184188
errors: [{ message: /looks up the Symbol\.split property/ }],
185189
},
190+
{
191+
code: 'new Proxy({}, {})',
192+
errors: [{ message: /null-prototype/ }]
193+
},
194+
{
195+
code: 'new Proxy({}, { [`__proto__`]: null })',
196+
errors: [{ message: /null-prototype/ }]
197+
},
198+
{
199+
code: 'new Proxy({}, { __proto__: Object.prototype })',
200+
errors: [{ message: /null-prototype/ }]
201+
},
202+
{
203+
code: 'new Proxy({}, { ...{ __proto__: null } })',
204+
errors: [{ message: /null-prototype/ }]
205+
},
186206
]
187207
});

‎tools/eslint-rules/avoid-prototype-pollution.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,23 @@ module.exports = {
128128
...createUnsafeStringMethodReport(context, 'StringPrototypeReplaceAll', 'Symbol.replace'),
129129
...createUnsafeStringMethodReport(context, 'StringPrototypeSearch', 'Symbol.search'),
130130
...createUnsafeStringMethodReport(context, 'StringPrototypeSplit', 'Symbol.split'),
131+
132+
'NewExpression[callee.name="Proxy"][arguments.1.type="ObjectExpression"]'(node) {
133+
for (const { key, value } of node.arguments[1].properties) {
134+
if (
135+
key != null && value != null &&
136+
((key.type === 'Identifier' && key.name === '__proto__') ||
137+
(key.type === 'Literal' && key.value === '__proto__')) &&
138+
value.type === 'Literal' && value.value === null
139+
) {
140+
return;
141+
}
142+
}
143+
context.report({
144+
node,
145+
message: 'Proxy handler must be a null-prototype object'
146+
});
147+
}
131148
};
132149
},
133150
};

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL