tools,doc: add guards against prototype pollution when creating proxies

PR-URL: https://github.com/nodejs/node/pull/43391
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
This commit is contained in:
Antoine du Hamel 2022-06-12 13:52:43 +02:00
parent 9119382555
commit 358008f473
No known key found for this signature in database
GPG Key ID: 21D900FFDB233756
6 changed files with 67 additions and 0 deletions

View File

@ -705,3 +705,27 @@ class SomeClass {
ObjectDefineProperty(SomeClass.prototype, 'readOnlyProperty', kEnumerableProperty); ObjectDefineProperty(SomeClass.prototype, 'readOnlyProperty', kEnumerableProperty);
console.log(new SomeClass().readOnlyProperty); // genuine data console.log(new SomeClass().readOnlyProperty); // genuine data
``` ```
### Defining a `Proxy` handler
When defining a `Proxy`, the handler object could be at risk of prototype
pollution when using a plain object literal:
```js
// User-land
Object.prototype.get = () => 'Unrelated user-provided data';
// Core
const objectToProxy = { someProperty: 'genuine value' };
const proxyWithPlainObjectLiteral = new Proxy(objectToProxy, {
has() { return false; },
});
console.log(proxyWithPlainObjectLiteral.someProperty); // Unrelated user-provided data
const proxyWithNullPrototypeObject = new Proxy(objectToProxy, {
__proto__: null,
has() { return false; },
});
console.log(proxyWithNullPrototypeObject.someProperty); // genuine value
```

View File

@ -117,6 +117,7 @@ function createAgentProxy(domain, client) {
}; };
return new Proxy(agent, { return new Proxy(agent, {
__proto__: null,
get(target, name) { get(target, name) {
if (name in target) return target[name]; if (name in target) return target[name];
return function callVirtualMethod(params) { return function callVirtualMethod(params) {

View File

@ -987,6 +987,7 @@ function trackAssignmentsTypedArray(typedArray) {
} }
return new Proxy(typedArray, { return new Proxy(typedArray, {
__proto__: null,
get(obj, prop, receiver) { get(obj, prop, receiver) {
if (prop === 'copyAssigned') { if (prop === 'copyAssigned') {
return copyAssigned; return copyAssigned;

View File

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

View File

@ -45,6 +45,10 @@ new RuleTester({
'ReflectDefineProperty({}, "key", { "__proto__": null })', 'ReflectDefineProperty({}, "key", { "__proto__": null })',
'ObjectDefineProperty({}, "key", { \'__proto__\': null })', 'ObjectDefineProperty({}, "key", { \'__proto__\': null })',
'ReflectDefineProperty({}, "key", { \'__proto__\': null })', 'ReflectDefineProperty({}, "key", { \'__proto__\': null })',
'new Proxy({}, otherObject)',
'new Proxy({}, someFactory())',
'new Proxy({}, { __proto__: null })',
'new Proxy({}, { __proto__: null, ...{} })',
], ],
invalid: [ invalid: [
{ {
@ -183,5 +187,21 @@ new RuleTester({
code: 'StringPrototypeSplit("some string", /some regex/)', code: 'StringPrototypeSplit("some string", /some regex/)',
errors: [{ message: /looks up the Symbol\.split property/ }], errors: [{ message: /looks up the Symbol\.split property/ }],
}, },
{
code: 'new Proxy({}, {})',
errors: [{ message: /null-prototype/ }]
},
{
code: 'new Proxy({}, { [`__proto__`]: null })',
errors: [{ message: /null-prototype/ }]
},
{
code: 'new Proxy({}, { __proto__: Object.prototype })',
errors: [{ message: /null-prototype/ }]
},
{
code: 'new Proxy({}, { ...{ __proto__: null } })',
errors: [{ message: /null-prototype/ }]
},
] ]
}); });

View File

@ -128,6 +128,23 @@ module.exports = {
...createUnsafeStringMethodReport(context, 'StringPrototypeReplaceAll', 'Symbol.replace'), ...createUnsafeStringMethodReport(context, 'StringPrototypeReplaceAll', 'Symbol.replace'),
...createUnsafeStringMethodReport(context, 'StringPrototypeSearch', 'Symbol.search'), ...createUnsafeStringMethodReport(context, 'StringPrototypeSearch', 'Symbol.search'),
...createUnsafeStringMethodReport(context, 'StringPrototypeSplit', 'Symbol.split'), ...createUnsafeStringMethodReport(context, 'StringPrototypeSplit', 'Symbol.split'),
'NewExpression[callee.name="Proxy"][arguments.1.type="ObjectExpression"]'(node) {
for (const { key, value } of node.arguments[1].properties) {
if (
key != null && value != null &&
((key.type === 'Identifier' && key.name === '__proto__') ||
(key.type === 'Literal' && key.value === '__proto__')) &&
value.type === 'Literal' && value.value === null
) {
return;
}
}
context.report({
node,
message: 'Proxy handler must be a null-prototype object'
});
}
}; };
}, },
}; };