Skip to content

package "pass-style" should not treat document.all as undefined #3156

@gibson042

Description

@gibson042

Describe the bug

isPrimitive functions, including the one exported by package "pass-style", are currently defined like

export const isPrimitive = val =>
  // Safer would be `Object(val) !== val` but is too expensive on XS.
  // So instead we use this adhoc set of type tests. But this is not safe in
  // the face of possible evolution of the language. Beware!
  !val || (typeof val !== 'object' && typeof val !== 'function');

And passStyleOf itself is fundamentally a switch on the result of typeof in which "undefined" is unconditionally treated as corresponding with the eponymous pass style.

Both of these implementations are incorrect for document.all, which [uniquely] implements HTMLAllCollection and thereby the [[IsHTMLDDA]] Internal Slot that causes ToBoolean output to be false, loose equality comparison against null or undefined to be true, and typeof output to be "undefined".

Consequently, document.all is incorrectly treated as a passable primitive undefined.

Steps to reproduce

document.all is limited to web browsers, so this reproduction is run there rather than in Node.js: https://jsfiddle.net/px4uwLdy/

<script type="module">
  import 'https://esm.run/@endo/init@1.1.13';
  import { isPrimitive, passStyleOf } from 'https://esm.run/@endo/pass-style@1.7.0';
  console.log(`isPrimitive: ${isPrimitive(document.all)}`);
  console.log(`passStyleOf: ${passStyleOf(document.all)}`);
</script>

Expected behavior

Console log output of "isPrimitive: false", followed by a "Cannot pass non-frozen objects" error.

Actual behavior

Console log output of "isPrimitive: true" and then "passStyleOf: undefined".

Platform environment

A web browser.

Additional context

HTMLAllCollection is a legacy platform object with a [[PreventExtensions]] internal method that unconditionally returns false, preventing document.all from being frozen. So the object-handling branch of passStyleOf is already prepared to throw an error when isFrozen returns false.

Further, pass-style is effectively our fundamental defensible boundary—if it correctly handles document.all, then other code which still treats it as undefined has a much smaller exploitable surface area and blast radius.

Suggested fix

Note that while document.all == undefined (loose equality) is true, document.all === undefined (strict equality) is false.

packages/pass-style/src/passStyle-helpers.js

 export const isPrimitive = val =>
   // Safer would be `Object(val) !== val` but is too expensive on XS.
   // So instead we use this adhoc set of type tests. But this is not safe in
-  // the face of possible evolution of the language. Beware!
+  // the face of possible evolution of the language, and already includes
+  // special logic for accepting `null` and `undefined` but not
+  // `document.all`. Beware!
-  !val || (typeof val !== 'object' && typeof val !== 'function');
+  val != null
+    ? typeof val !== 'object' && typeof val !== 'function'
+    : val === null || val === undefined;

packages/pass-style/src/passStyleOf.js

     const passStyleOfInternal = inner => {
-      const typestr = typeof inner;
+      // eslint-disable-next-line no-nested-ternary
+      const typestr =
+        inner != null
+          ? typeof inner
+          : inner === null
+            ? 'null'
+            : inner === undefined
+              ? 'undefined'
+              // probably document.all
+              : 'object';
       switch (typestr) {
+        case 'null':
         case 'undefined':
         case 'boolean':
         case 'number':
         case 'bigint': {
           return typestr;
         }
         case 'string': {
           assertPassableString(inner);
           return 'string';
         }
         case 'symbol': {
           assertPassableSymbol(inner);
           return 'symbol';
         }
         case 'object': {
-          if (inner === null) {
-            return 'null';
-          }
           if (!isFrozen(inner)) {

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions