Skip to content

Commit

Permalink
util: prevent tampering with internals in inspect()
Browse files Browse the repository at this point in the history
This makes sure user options passed to `util.inspect()` will not
override any internal properties (besides `stylize`).

PR-URL: #26577
Fixes: #24765
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
BridgeAR committed Mar 13, 2019
1 parent 32853c0 commit 5672ab7
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 9 deletions.
28 changes: 21 additions & 7 deletions lib/internal/util/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,16 @@ const meta = [
'', '', '', '', '', '', '', '\\\\'
];

function getUserOptions(ctx) {
const obj = { stylize: ctx.stylize };
for (const key of Object.keys(inspectDefaultOptions)) {
obj[key] = ctx[key];
}
if (ctx.userOptions === undefined)
return obj;
return { ...obj, ...ctx.userOptions };
}

/**
* Echos the value of any input. Tries to print the value out
* in the best way possible given the different types.
Expand Down Expand Up @@ -192,8 +202,16 @@ function inspect(value, opts) {
ctx.showHidden = opts;
} else if (opts) {
const optKeys = Object.keys(opts);
for (var i = 0; i < optKeys.length; i++) {
ctx[optKeys[i]] = opts[optKeys[i]];
for (const key of optKeys) {
// TODO(BridgeAR): Find a solution what to do about stylize. Either make
// this function public or add a new API with a similar or better
// functionality.
if (hasOwnProperty(inspectDefaultOptions, key) || key === 'stylize') {
ctx[key] = opts[key];
} else if (ctx.userOptions === undefined) {
// This is required to pass through the actual user input.
ctx.userOptions = opts;
}
}
}
}
Expand Down Expand Up @@ -522,14 +540,10 @@ function formatValue(ctx, value, recurseTimes, typedArray) {
maybeCustom !== inspect &&
// Also filter out any prototype objects using the circular check.
!(value.constructor && value.constructor.prototype === value)) {
// Remove some internal properties from the options before passing it
// through to the user function. This also prevents option manipulation.
// eslint-disable-next-line no-unused-vars
const { budget, seen, indentationLvl, ...plainCtx } = ctx;
// This makes sure the recurseTimes are reported as before while using
// a counter internally.
const depth = ctx.depth === null ? null : ctx.depth - recurseTimes;
const ret = maybeCustom.call(context, depth, plainCtx);
const ret = maybeCustom.call(context, depth, getUserOptions(ctx));
// If the custom inspection method returned `this`, don't go into
// infinite recursion.
if (ret !== context) {
Expand Down
12 changes: 10 additions & 2 deletions test/parallel/test-util-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ util.inspect({ hasOwnProperty: null });
}) };
});

util.inspect(subject, { customInspectOptions: true });
util.inspect(subject);

// util.inspect.custom is a shared symbol which can be accessed as
// Symbol.for("nodejs.util.inspect.custom").
Expand All @@ -803,9 +803,11 @@ util.inspect({ hasOwnProperty: null });

subject[inspect] = (depth, opts) => {
assert.strictEqual(opts.customInspectOptions, true);
assert.strictEqual(opts.seen, null);
return {};
};

util.inspect(subject, { customInspectOptions: true });
util.inspect(subject, { customInspectOptions: true, seen: null });
}

{
Expand All @@ -816,6 +818,12 @@ util.inspect({ hasOwnProperty: null });
`{ a: 123,\n [Symbol(${UIC})]: [Function: [${UIC}]] }`);
}

// Verify that it's possible to use the stylize function to manipulate input.
assert.strictEqual(
util.inspect([1, 2, 3], { stylize() { return 'x'; } }),
'[ x, x, x ]'
);

// Using `util.inspect` with "colors" option should produce as many lines as
// without it.
{
Expand Down

0 comments on commit 5672ab7

Please sign in to comment.