Skip to content

Commit

Permalink
tools,doc: add guards against prototype pollution when creating proxies
Browse files Browse the repository at this point in the history
PR-URL: #43391
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
  • Loading branch information
aduh95 authored and danielleadams committed Jun 16, 2022
1 parent ac9599a commit 812140c
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 0 deletions.
24 changes: 24 additions & 0 deletions doc/contributing/primordials.md
Original file line number Diff line number Diff line change
Expand Up @@ -705,3 +705,27 @@ class SomeClass {
ObjectDefineProperty(SomeClass.prototype, 'readOnlyProperty', kEnumerableProperty);
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
```
1 change: 1 addition & 0 deletions lib/internal/debugger/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ function createAgentProxy(domain, client) {
};

return new Proxy(agent, {
__proto__: null,
get(target, name) {
if (name in target) return target[name];
return function callVirtualMethod(params) {
Expand Down
1 change: 1 addition & 0 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,7 @@ function trackAssignmentsTypedArray(typedArray) {
}

return new Proxy(typedArray, {
__proto__: null,
get(obj, prop, receiver) {
if (prop === 'copyAssigned') {
return copyAssigned;
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ const wrapper = [
];

let wrapperProxy = new Proxy(wrapper, {
__proto__: null,

set(target, property, value, receiver) {
patched = true;
return ReflectSet(target, property, value, receiver);
Expand Down Expand Up @@ -718,6 +720,8 @@ function emitCircularRequireWarning(prop) {
// A Proxy that can be used as the prototype of a module.exports object and
// warns when non-existent properties are accessed.
const CircularRequirePrototypeWarningProxy = new Proxy({}, {
__proto__: null,

get(target, prop) {
// Allow __esModule access in any case because it is used in the output
// of transpiled code to determine whether something comes from an
Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-eslint-avoid-prototype-pollution.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ new RuleTester({
'ReflectDefineProperty({}, "key", { "__proto__": null })',
'ObjectDefineProperty({}, "key", { \'__proto__\': null })',
'ReflectDefineProperty({}, "key", { \'__proto__\': null })',
'new Proxy({}, otherObject)',
'new Proxy({}, someFactory())',
'new Proxy({}, { __proto__: null })',
'new Proxy({}, { __proto__: null, ...{} })',
],
invalid: [
{
Expand Down Expand Up @@ -183,5 +187,21 @@ new RuleTester({
code: 'StringPrototypeSplit("some string", /some regex/)',
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/ }]
},
]
});
17 changes: 17 additions & 0 deletions tools/eslint-rules/avoid-prototype-pollution.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,23 @@ module.exports = {
...createUnsafeStringMethodReport(context, 'StringPrototypeReplaceAll', 'Symbol.replace'),
...createUnsafeStringMethodReport(context, 'StringPrototypeSearch', 'Symbol.search'),
...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'
});
}
};
},
};

0 comments on commit 812140c

Please sign in to comment.