lib: Add option to disable proto accessors by devsnek · Pull Request #32279 · nodejs/node · GitHub

Adds --disable-proto CLI option which can be set to delete or
throw.

Fixes #31951

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Maybe clarify somewhere that this only affects the main Context?

lgtm

Does this also impact worker threads? I think it should and we could also add an option there.

I'm also perfectly fine in landing this as this and opening an issue for worker threads etc.

Does this also impact worker threads? I think it should and we could also add an option there.

Worker threads inherit their execArgv from the parent thread, unless specified otherwise. I would not add a separate option for this.

Does it feasible to add an option to ignore the __proto__ field rather than throwing? If the engine does ignore the __proto__s there is no reason for users to handle these errors.

@legendecas How would that differ from --disable-proto=delete? Are you suggesting that a.__proto__ = {}; does nothing, including not setting __proto__ as a regular property? That would seem more surprising to me…

@addaleax I tried the PR locally, it doesn't seem to be effective on the case of delete since only Object.prototype.__proto__ is deleted in the PR:

$ node --disable-proto=delete -p '({ __proto__: Array.prototype }).push === Array.prototype.push'
true

What I'm understanding/expecting is:

$ node --disable-proto=delete -p '({ __proto__: Array.prototype }).push'
undefined

@legendecas object literals do not use the getter/setter and are a special case of syntax. They are not problematic for assignment like the accessors and not part of the CVEs in question

This should update doc/node.1 too.

@cjihrig can you verify i did the node.1 change correctly? i just kind of guessed based on other entries...

@devsnek you can run man doc/node.1 to look at the finished product.

EDIT: It looks good at a quick glance, but I don't really know the format and need to view it with man.

The given solution might not provide protection against the following test case:

"use strict";

const assert = require('assert');
const vm = require('vm');

(function patchProto() {
	const protoThrow = () => {throw new Error(`Illegal __proto__ access`)}
	Object.defineProperty(Object.prototype, '__proto__', {get: protoThrow, set: protoThrow, enumerable: false, configurable: false});
})();

function getCrossRealmObject() {
	const ctx = vm.createContext({});
	vm.runInContext('value = {"foreign":1}', ctx);
	return ctx.value;
}

function testProto() {
	const payload = '{"__proto__": {"evil": true}}';

	const x = getCrossRealmObject();
	try {
		Object.assign(x, JSON.parse(payload));
	} catch (err) {}
	assert(!x.evil);
}

testProto();

If it does, please add such a test!

@Slayer95 See @addaleax's #32279 (comment):

this only affects the main Context

Adds `--disable-proto` CLI option which can be set to `delete` or `throw`. Fixes nodejs#31951 PR-URL: nodejs#32279 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>

Adds `--disable-proto` CLI option which can be set to `delete` or `throw`. Fixes #31951 PR-URL: #32279 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>

macOS package notarization and a change in builder configuration: The macOS binaries for this release, and future 13.x releases, are now being compiled on macOS 10.15 (Catalina) with Xcode 11 to support package notarization, a requirement for installing on .pkg files on macOS 10.15 and later. Previous builds of Node.js 13.x were compiled on macOS 10.11 (El Capitan) with Xcode 10. As binaries are still being compiled to support a minimum of macOS 10.10 (Yosemite) we do not anticipate this having a negative impact on Node.js 13.x users with older versions of macOS. Notable changes: * build: * macOS package notarization (Rod Vagg) #31459 * deps: * update npm to 6.14.3 (Myles Borins) #32368 * update to uvwasi 0.0.6 (Colin Ihrig) #32309 * upgrade to libuv 1.35.0 (Colin Ihrig) #32204 * lib: * add --disable-proto option to cli (Gus Caplan) #32279 * node_report: * move diagnostic reports to stable (Colin Ihrig) #32242 * worker: * allow URL in Worker constructor (Antoine du HAMEL) #31664 * util: * use a global symbol for `util.promisify.custom` (ExE Boss) #31672 PR-URL: #32376

macOS package notarization and a change in builder configuration: The macOS binaries for this release, and future 13.x releases, are now being compiled on macOS 10.15 (Catalina) with Xcode 11 to support package notarization, a requirement for installing on .pkg files on macOS 10.15 and later. Previous builds of Node.js 13.x were compiled on macOS 10.11 (El Capitan) with Xcode 10. As binaries are still being compiled to support a minimum of macOS 10.10 (Yosemite) we do not anticipate this having a negative impact on Node.js 13.x users with older versions of macOS. Notable changes: * build: * macOS package notarization (Rod Vagg) #31459 * deps: * upgrade npm to 6.14.4 (Ruy Adorno) #32495 * update to uvwasi 0.0.6 (Colin Ihrig) #32309 * upgrade to libuv 1.35.0 (Colin Ihrig) #32204 * lib: * add --disable-proto option to cli (Gus Caplan) #32279 * node_report: * move diagnostic reports to stable (Colin Ihrig) #32242 * worker: * allow URL in Worker constructor (Antoine du HAMEL) #31664 * util: * use a global symbol for `util.promisify.custom` (ExE Boss) #31672 PR-URL: #32376

Adds `--disable-proto` CLI option which can be set to `delete` or `throw`. Fixes nodejs#31951 PR-URL: nodejs#32279 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>

Adds `--disable-proto` CLI option which can be set to `delete` or `throw`. Fixes #31951 PR-URL: #32279 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>

devsnek added the security Issues and PRs related to security. label Mar 15, 2020

devsnek requested review from bmeck, guybedford, jasnell and mcollina March 15, 2020 01:04

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Mar 15, 2020

devsnek force-pushed the disable-proto branch 3 times, most recently from 39a58eb to 8a31826 Compare March 15, 2020 02:27

mscdex reviewed Mar 15, 2020

View reviewed changes

Comment thread lib/internal/bootstrap/pre_execution.js Outdated

mcollina approved these changes Mar 15, 2020

View reviewed changes

tniessen reviewed Mar 15, 2020

View reviewed changes

Comment thread doc/api/errors.md Outdated

cjihrig reviewed Mar 15, 2020

View reviewed changes

devnexen approved these changes Mar 15, 2020

View reviewed changes

devsnek force-pushed the disable-proto branch from 8a31826 to 1542ae5 Compare March 15, 2020 21:20

devsnek force-pushed the disable-proto branch 2 times, most recently from 7489319 to 5559f5f Compare March 15, 2020 23:10

bmeck changed the title lib: Add option to disable __proto__ lib: Add option to disable __proto__ accessors Mar 16, 2020

bmeck reviewed Mar 16, 2020

View reviewed changes

Comment thread doc/api/cli.md Outdated

bmeck approved these changes Mar 17, 2020

View reviewed changes

legendecas approved these changes Mar 17, 2020

View reviewed changes

devnexen approved these changes Mar 17, 2020

View reviewed changes

vdeturckheim approved these changes Mar 17, 2020

View reviewed changes

devsnek force-pushed the disable-proto branch from 3dca536 to 7a742ec Compare March 18, 2020 17:23

devsnek merged commit 7a742ec into nodejs:master Mar 18, 2020

devsnek deleted the disable-proto branch March 18, 2020 17:24

MylesBorins mentioned this pull request Mar 19, 2020

v13.12.0 proposal #32376

Merged

bmeck mentioned this pull request Mar 20, 2020

Use Object.setPrototypeOf as __proto__ could potentially be disabled chalk/chalk#387

Merged

devsnek added the notable-change PRs with changes that should be highlighted in changelogs. label Mar 23, 2020

This was referenced Apr 16, 2020

remove use of __proto__ acornjs/acorn-private-class-elements#13

Closed

support node --disable-proto rollup/rollup#3500

Closed

support node --disable-proto rollup/rollup#3501

Closed

targos added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 22, 2020

madarche mentioned this pull request Apr 22, 2020

Feat switch to jest mozilla/node-convict#365

Merged

targos mentioned this pull request May 2, 2020

v12.17.0 release proposal #33197

Merged

This was referenced Jan 22, 2021

Avoid hitting __proto__ isaacs/node-graceful-fs#201

Closed

Avoid hitting __proto__ in _inheritsLoose babel/babel#12693

Merged

coderabbitai Bot mentioned this pull request Aug 5, 2026

chore: merge v11.16.1 back to the develop branch mermaid-js/mermaid#8029

Merged

4 tasks

lib: Add option to disable __proto__ accessors by devsnek · Pull Request #32279 · nodejs/node · GitHub

Uh oh!

Uh oh!

lib: Add option to disable __proto__ accessors - #32279

lib: Add option to disable __proto__ accessors#32279devsnek merged 1 commit intonodejs:masternodejs/node:masterfrom devsnek:disable-protodevsnek/node:disable-protoCopy head branch name to clipboard

Conversation

devsnek commented Mar 15, 2020

Checklist

Uh oh!

Uh oh!

addaleax commented Mar 15, 2020

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

addaleax commented Mar 15, 2020

Uh oh!

legendecas commented Mar 15, 2020

Uh oh!

addaleax commented Mar 15, 2020

Uh oh!

legendecas commented Mar 15, 2020

Uh oh!

Uh oh!

bmeck commented Mar 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

devsnek commented Mar 15, 2020

Uh oh!

cjihrig commented Mar 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Slayer95 commented Mar 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

tniessen commented Mar 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

lib: Add option to disable proto accessors by devsnek · Pull Request #32279 · nodejs/node · GitHub

lib: Add option to disable proto accessors - #32279

lib: Add option to disable proto accessors#32279
devsnek merged 1 commit into
nodejs:masterfrom
devsnek:disable-proto

bmeck commented Mar 15, 2020 •
edited
Loading

cjihrig commented Mar 15, 2020 •
edited
Loading

Slayer95 commented Mar 16, 2020 •
edited
Loading