From 6262973769cd8d7cf8f3fa70a53d496402423396 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 28 Dec 2020 12:20:05 +0100 Subject: [PATCH 1/3] process: disallow adding options to process.allowedNodeEnvironmentFlags Make no-op direct calls of `Set` prototype methods to `process.allowedNodeEnvironmentFlags`. ```js const { add } = Set.prototype; add.call(process.allowedNodeEnvironmentFlags, '--user-option`); process.allowedNodeEnvironmentFlags.has('--user-option') === false; ``` --- lib/internal/process/per_thread.js | 58 +++++++++++++------ .../test-process-env-allowed-flags.js | 38 ++++++++++-- 2 files changed, 73 insertions(+), 23 deletions(-) diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index 04da1a862ca7c0..8c89709e897034 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -6,21 +6,24 @@ const { ArrayPrototypeEvery, + ArrayPrototypeIncludes, ArrayPrototypeMap, ArrayPrototypePush, ArrayPrototypeSplice, BigUint64Array, Float64Array, NumberMAX_SAFE_INTEGER, - ObjectDefineProperty, ObjectFreeze, ReflectApply, RegExpPrototypeTest, - SafeSet, + SafeArrayIterator, + Set, StringPrototypeEndsWith, StringPrototypeReplace, StringPrototypeSlice, StringPrototypeStartsWith, + Symbol, + SymbolIterator, Uint32Array, } = primordials; @@ -41,6 +44,8 @@ const { } = require('internal/validators'); const constants = internalBinding('constants').os.signals; +const kSet = Symbol('internal set'); + function assert(x, msg) { if (!x) throw new ERR_ASSERTION(msg || 'assertion error'); } @@ -293,18 +298,18 @@ function buildAllowedFlags() { // Save these for comparison against flags provided to // process.allowedNodeEnvironmentFlags.has() which lack leading dashes. - const nodeFlags = new SafeSet(ArrayPrototypeMap(allowedNodeEnvironmentFlags, - trimLeadingDashes)); - - class NodeEnvironmentFlagsSet extends SafeSet { - constructor(...args) { - super(...args); - - // The super constructor consumes `add`, but - // disallow any future adds. - ObjectDefineProperty(this, 'add', { - value: () => this - }); + const nodeFlags = ArrayPrototypeMap(allowedNodeEnvironmentFlags, + trimLeadingDashes); + + class NodeEnvironmentFlagsSet extends Set { + constructor(iterable) { + super(); + this[kSet] = new Set(new SafeArrayIterator(iterable)); + } + + add() { + // No-op, `Set` API compatible + return this; } delete() { @@ -313,7 +318,7 @@ function buildAllowedFlags() { } clear() { - // No-op + // No-op, `Set` API compatible } has(key) { @@ -328,13 +333,32 @@ function buildAllowedFlags() { key = StringPrototypeReplace(key, replaceUnderscoresRegex, '-'); if (RegExpPrototypeTest(leadingDashesRegex, key)) { key = StringPrototypeReplace(key, trailingValuesRegex, ''); - return super.has(key); + return this[kSet].has(key); } - return nodeFlags.has(key); + return ArrayPrototypeIncludes(nodeFlags, key); } return false; } + + entries() { + return this[kSet].entries(); + } + + forEach(callback, thisArg = undefined) { + this[kSet].forEach((v) => ReflectApply(callback, thisArg, [v, v, this])); + } + + get size() { + return this[kSet].size; + } + + values() { + return this[kSet].values(); + } } + NodeEnvironmentFlagsSet.prototype.keys = + NodeEnvironmentFlagsSet.prototype[SymbolIterator] = + NodeEnvironmentFlagsSet.prototype.values; ObjectFreeze(NodeEnvironmentFlagsSet.prototype.constructor); ObjectFreeze(NodeEnvironmentFlagsSet.prototype); diff --git a/test/parallel/test-process-env-allowed-flags.js b/test/parallel/test-process-env-allowed-flags.js index 21beb5357a5f65..5843985ca692af 100644 --- a/test/parallel/test-process-env-allowed-flags.js +++ b/test/parallel/test-process-env-allowed-flags.js @@ -1,6 +1,6 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); // Assert legit flags are allowed, and bogus flags are disallowed @@ -62,15 +62,41 @@ const assert = require('assert'); true); process.allowedNodeEnvironmentFlags.add('foo'); + Set.prototype.add.call(process.allowedNodeEnvironmentFlags, 'foo'); assert.strictEqual(process.allowedNodeEnvironmentFlags.has('foo'), false); - process.allowedNodeEnvironmentFlags.forEach((flag) => { - assert.strictEqual(flag === 'foo', false); - }); - process.allowedNodeEnvironmentFlags.clear(); - assert.strictEqual(process.allowedNodeEnvironmentFlags.size > 0, true); + const thisArg = {}; + process.allowedNodeEnvironmentFlags.forEach( + common.mustCallAtLeast(function(flag, _, set) { + assert.notStrictEqual(flag, 'foo'); + assert.strictEqual(this, thisArg); + assert.strictEqual(set, process.allowedNodeEnvironmentFlags); + }), + thisArg + ); + + for (const flag of process.allowedNodeEnvironmentFlags.keys()) { + assert.notStrictEqual(flag, 'foo'); + } + for (const flag of process.allowedNodeEnvironmentFlags.values()) { + assert.notStrictEqual(flag, 'foo'); + } + for (const flag of process.allowedNodeEnvironmentFlags) { + assert.notStrictEqual(flag, 'foo'); + } + for (const [flag] of process.allowedNodeEnvironmentFlags.entries()) { + assert.notStrictEqual(flag, 'foo'); + } const size = process.allowedNodeEnvironmentFlags.size; + + process.allowedNodeEnvironmentFlags.clear(); + assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size); + Set.prototype.clear.call(process.allowedNodeEnvironmentFlags); + assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size); + process.allowedNodeEnvironmentFlags.delete('-r'); assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size); + Set.prototype.delete.call(process.allowedNodeEnvironmentFlags, '-r'); + assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size); } From eafaa2b99b61147c2659eab887d2ba67ed6fab04 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 1 Feb 2021 20:19:26 +0100 Subject: [PATCH 2/3] fixup! process: disallow adding options to process.allowedNodeEnvironmentFlags --- lib/internal/process/per_thread.js | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index 8c89709e897034..0e1dbafc08f289 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -6,6 +6,7 @@ const { ArrayPrototypeEvery, + ArrayPrototypeForEach, ArrayPrototypeIncludes, ArrayPrototypeMap, ArrayPrototypePush, @@ -18,6 +19,8 @@ const { RegExpPrototypeTest, SafeArrayIterator, Set, + SetPrototypeEntries, + SetPrototypeValues, StringPrototypeEndsWith, StringPrototypeReplace, StringPrototypeSlice, @@ -44,7 +47,7 @@ const { } = require('internal/validators'); const constants = internalBinding('constants').os.signals; -const kSet = Symbol('internal set'); +const kInternal = Symbol('internal properties'); function assert(x, msg) { if (!x) throw new ERR_ASSERTION(msg || 'assertion error'); @@ -302,9 +305,9 @@ function buildAllowedFlags() { trimLeadingDashes); class NodeEnvironmentFlagsSet extends Set { - constructor(iterable) { + constructor(array) { super(); - this[kSet] = new Set(new SafeArrayIterator(iterable)); + this[kInternal] = { array }; } add() { @@ -333,7 +336,7 @@ function buildAllowedFlags() { key = StringPrototypeReplace(key, replaceUnderscoresRegex, '-'); if (RegExpPrototypeTest(leadingDashesRegex, key)) { key = StringPrototypeReplace(key, trailingValuesRegex, ''); - return this[kSet].has(key); + return ArrayPrototypeIncludes(this[kInternal].array, key); } return ArrayPrototypeIncludes(nodeFlags, key); } @@ -341,19 +344,26 @@ function buildAllowedFlags() { } entries() { - return this[kSet].entries(); + this[kInternal].set ??= + new Set(new SafeArrayIterator(this[kInternal].array)); + return SetPrototypeEntries(this[kInternal].set); } forEach(callback, thisArg = undefined) { - this[kSet].forEach((v) => ReflectApply(callback, thisArg, [v, v, this])); + ArrayPrototypeForEach( + this[kInternal].array, + (v) => ReflectApply(callback, thisArg, [v, v, this]) + ); } get size() { - return this[kSet].size; + return this[kInternal].array.length; } values() { - return this[kSet].values(); + this[kInternal].set ??= + new Set(new SafeArrayIterator(this[kInternal].array)); + return SetPrototypeValues(this[kInternal].set); } } NodeEnvironmentFlagsSet.prototype.keys = From 466d928b60309014cf079b225aaf52e48958c08d Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 17 Feb 2021 15:34:57 +0100 Subject: [PATCH 3/3] fixup! fixup! process: disallow adding options to process.allowedNodeEnvironmentFlags --- test/parallel/test-process-env-allowed-flags.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-process-env-allowed-flags.js b/test/parallel/test-process-env-allowed-flags.js index 5843985ca692af..66be07b8b872c2 100644 --- a/test/parallel/test-process-env-allowed-flags.js +++ b/test/parallel/test-process-env-allowed-flags.js @@ -62,6 +62,7 @@ const assert = require('assert'); true); process.allowedNodeEnvironmentFlags.add('foo'); + assert.strictEqual(process.allowedNodeEnvironmentFlags.has('foo'), false); Set.prototype.add.call(process.allowedNodeEnvironmentFlags, 'foo'); assert.strictEqual(process.allowedNodeEnvironmentFlags.has('foo'), false);