From f1753d4c7662025c83e70906ac7d3812a34feb16 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 28 Dec 2020 12:20:05 +0100 Subject: [PATCH] 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; ``` PR-URL: https://github.com/nodejs/node/pull/36660 Reviewed-By: Darshan Sen Reviewed-By: Rich Trott Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell --- lib/internal/process/per_thread.js | 68 ++++++++++++++----- .../test-process-env-allowed-flags.js | 39 +++++++++-- 2 files changed, 84 insertions(+), 23 deletions(-) diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index a4da131a0cea77..e85029735a2c42 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -6,21 +6,27 @@ const { ArrayPrototypeEvery, + ArrayPrototypeForEach, + ArrayPrototypeIncludes, ArrayPrototypeMap, ArrayPrototypePush, ArrayPrototypeSplice, BigUint64Array, Float64Array, NumberMAX_SAFE_INTEGER, - ObjectDefineProperty, ObjectFreeze, ReflectApply, RegExpPrototypeTest, - SafeSet, + SafeArrayIterator, + Set, + SetPrototypeEntries, + SetPrototypeValues, StringPrototypeEndsWith, StringPrototypeReplace, StringPrototypeSlice, StringPrototypeStartsWith, + Symbol, + SymbolIterator, Uint32Array, } = primordials; @@ -41,6 +47,8 @@ const { } = require('internal/validators'); const constants = internalBinding('constants').os.signals; +const kInternal = Symbol('internal properties'); + function assert(x, msg) { if (!x) throw new ERR_ASSERTION(msg || 'assertion error'); } @@ -293,18 +301,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(array) { + super(); + this[kInternal] = { array }; + } + + add() { + // No-op, `Set` API compatible + return this; } delete() { @@ -313,7 +321,7 @@ function buildAllowedFlags() { } clear() { - // No-op + // No-op, `Set` API compatible } has(key) { @@ -328,13 +336,39 @@ function buildAllowedFlags() { key = StringPrototypeReplace(key, replaceUnderscoresRegex, '-'); if (RegExpPrototypeTest(leadingDashesRegex, key)) { key = StringPrototypeReplace(key, trailingValuesRegex, ''); - return super.has(key); + return ArrayPrototypeIncludes(this[kInternal].array, key); } - return nodeFlags.has(key); + return ArrayPrototypeIncludes(nodeFlags, key); } return false; } + + entries() { + this[kInternal].set ??= + new Set(new SafeArrayIterator(this[kInternal].array)); + return SetPrototypeEntries(this[kInternal].set); + } + + forEach(callback, thisArg = undefined) { + ArrayPrototypeForEach( + this[kInternal].array, + (v) => ReflectApply(callback, thisArg, [v, v, this]) + ); + } + + get size() { + return this[kInternal].array.length; + } + + values() { + this[kInternal].set ??= + new Set(new SafeArrayIterator(this[kInternal].array)); + return SetPrototypeValues(this[kInternal].set); + } } + 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..66be07b8b872c2 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 @@ -63,14 +63,41 @@ const assert = require('assert'); process.allowedNodeEnvironmentFlags.add('foo'); assert.strictEqual(process.allowedNodeEnvironmentFlags.has('foo'), false); - process.allowedNodeEnvironmentFlags.forEach((flag) => { - assert.strictEqual(flag === 'foo', false); - }); + Set.prototype.add.call(process.allowedNodeEnvironmentFlags, 'foo'); + assert.strictEqual(process.allowedNodeEnvironmentFlags.has('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); }