Tests for non-own property access

CHANGELOG.md

@@ -7,7 +7,7 @@ Versioning].
 
 ## [Unreleased]
 
-- _No changes yet_
+- Harden against polluted prototypes. ([#1280])
 
 ## [2.0.1] - 2023-10-28
 
@@ -318,6 +318,7 @@ Versioning].
 [#1137]: https://github.com/ericcornelissen/shescape/pull/1137
 [#1142]: https://github.com/ericcornelissen/shescape/pull/1142
 [#1149]: https://github.com/ericcornelissen/shescape/pull/1149
+[#1280]: https://github.com/ericcornelissen/shescape/pull/1280
 [552e8ea]: https://github.com/ericcornelissen/shescape/commit/552e8eab56861720b1d4e5474fb65741643358f9
 [keep a changelog]: https://keepachangelog.com/en/1.0.0/
 [semantic versioning]: https://semver.org/spec/v2.0.0.html

script/run-platform-coverage.js

@@ -15,4 +15,5 @@ if (!testType) {
   process.exit(1);
 }
 
-common.npm(["run", `coverage:${testType}:${platform}`]);
+const cmd = common.npm(["run", `coverage:${testType}:${platform}`]);
+cmd.on("close", (code) => process.exit(code));

src/options.js

@@ -24,14 +24,26 @@ function unsupportedError(shellName) {
   return `Shescape does not support the shell ${shellName}`;
 }
 
+/**
+ * Check if the given object has the given property as an own property.
+ *
+ * This custom function is used over `Object.hasOwn` because that isn't
+ * available in all supported Node.js versions.
+ *
+ * @param {object} object The object of interest.
+ * @param {string} property The property of interest.
+ * @returns {boolean} `true` if property is an own-property, `false` otherwise.
+ */
+function hasOwn(object, property) {
+  return Object.prototype.hasOwnProperty.call(object, property);
+}
+
 /**
  * Parses options provided to shescape.
  *
  * @param {object} args The arguments for this function.
  * @param {Object<string, string>} args.env The environment variables.
  * @param {object} args.options The options for escaping.
- * @param {boolean} [args.options.flagProtection] Is flag protection enabled.
- * @param {boolean | string} [args.options.shell=true] The shell to escape for.
  * @param {object} deps The dependencies for this function.
  * @param {Function} deps.getDefaultShell Function to get the default shell.
  * @param {Function} deps.getShellName Function to get the name of a shell.
@@ -40,9 +52,14 @@ function unsupportedError(shellName) {
  * @throws {Error} The shell is not supported or could not be found.
  */
 export function parseOptions(
-  { env, options: { flagProtection, shell } },
+  { env, options },
   { getDefaultShell, getShellName, isShellSupported },
 ) {
+  let flagProtection = hasOwn(options, "flagProtection")
+    ? options.flagProtection
+    : undefined;
+  let shell = hasOwn(options, "shell") ? options.shell : undefined;
+
   flagProtection =
     flagProtection === undefined ? true : flagProtection ? true : false;
 

test/integration/constructor/_.js

@@ -0,0 +1,9 @@
+/**
+ * @overview Provides testing utilities.
+ * @license MIT
+ */
+
+import * as arbitrary from "../../_arbitraries.js";
+import * as pollution from "./_pollution.js";
+
+export { arbitrary, pollution };

test/integration/constructor/_pollution.js

@@ -0,0 +1,98 @@
+/**
+ * @overview Contains helpers to detect non-own property access.
+ * @license MIT
+ */
+
+import assert from "node:assert/strict";
+
+/**
+ * A store for non-own property accesses by proxy.
+ */
+const pollutionState = new Map();
+
+/**
+ * Check whether or not a value is nil (`null` or `undefined`).
+ *
+ * @param {any} value The value to check.
+ * @returns {boolean} `true` if `value` is nil, `false` otherwise.
+ */
+function isNil(value) {
+  return value === null || value === undefined;
+}
+
+/**
+ * Check whether or not a value is a primitive value.
+ *
+ * @param {any} value The value to check.
+ * @returns {boolean} `true` if `value` is primitive, `false` otherwise.
+ */
+function isPrimitive(value) {
+  return typeof value === "number" || typeof value === "string";
+}
+
+/**
+ * Check whether or not a value can be wrapped by a `Proxy`.
+ *
+ * @param {any} value The value to check.
+ * @returns {boolean} `true` if `value` is proxyable, `false` otherwise.
+ */
+function isProxyable(value) {
+  return !(isNil(value) || isPrimitive(value));
+}
+
+/**
+ * Wrap the provided target in in order to monitor it for access to properties
+ * not present on the target.
+ *
+ * @param {any} target The value to wrap.
+ * @returns {any} The wrapped value (or original if wrapping isn't possible).
+ */
+export function wrap(target) {
+  if (!isProxyable(target)) {
+    return target;
+  }
+
+  const nonOwnAccesses = new Set();
+  const proxy = new Proxy(target, {
+    get(target, property, _proxy) {
+      if (!Object.hasOwn(target, property)) {
+        nonOwnAccesses.add(property);
+      }
+
+      // Return normal lookup to ensure normal test execution.
+      return target[property];
+    },
+  });
+
+  pollutionState.set(proxy, nonOwnAccesses);
+  return proxy;
+}
+
+/**
+ * Check if non-own property access was detected on the given wrapped object.
+ *
+ * @param {any} wrapped A `wrap`ped value.
+ * @throws {Error} If non-own property access was detected.
+ */
+export function check(wrapped) {
+  if (!isProxyable(wrapped)) {
+    return;
+  }
+
+  assert.ok(pollutionState.has(wrapped), "target not found");
+  const nonOwnAccesses = pollutionState.get(wrapped);
+
+  // Remove the proxy from the state so re-use of it does not result in errors
+  // from one test to affect other tests. (Also just to reduce memory usage.)
+  pollutionState.delete(wrapped);
+
+  const actual = nonOwnAccesses.size;
+  const expected = 0;
+  const propertiesList = Array.from(nonOwnAccesses.values()).join(", ");
+
+  assert.equal(
+    actual,
+    expected,
+    `Non-own access to ${actual} property(s) detected: ${propertiesList}`,
+  );
+}

test/integration/constructor/constructor.test.js

@@ -3,12 +3,28 @@
  * @license MIT
  */
 
+import { testProp } from "@fast-check/ava";
 import test from "ava";
 
+import { arbitrary, pollution } from "./_.js";
+
 import { Shescape } from "shescape";
 
 test("shell is unsupported", (t) => {
   const shell = "not-actually-a-shell-that-exists";
 
   t.throws(() => new Shescape({ shell }), { instanceOf: Error });
 });
+
+testProp(
+  "affected by prototype pollution",
+  [arbitrary.shescapeOptions().map(pollution.wrap)],
+  (t, options) => {
+    try {
+      new Shescape(options);
+    } catch (_) {}
+
+    pollution.check(options);
+    t.pass();
+  },
+);