fix: don't invoke user-defined prototype methods in primordials makeSafe

The `makeSafe` function in `primordials.js` was calling every zero-argument
function on `Set.prototype`/`Map.prototype` to check if it returns an
iterator. When user code added functions to these prototypes before
`import('fs')` triggered lazy primordials initialization, those user-defined
functions were invoked unexpectedly.

Fix by hardcoding the known iterator-returning method names (`entries`,
`keys`, `values`, `Symbol.iterator`) instead of dynamically probing all
prototype methods.

Closes #18890

Co-Authored-By: Claude <noreply@anthropic.com>

src/js/internal/primordials.js

@@ -44,26 +44,37 @@ const copyProps = (src, dest) => {
   });
 };
 
+// The known built-in iterator-returning method keys for Set and Map.
+// We hardcode these to avoid iterating user-added prototype methods,
+// which could have side effects when called. (see: #18890)
+const iteratorMethods = ["entries", "keys", "values", Symbol.iterator];
+
 const makeSafe = (unsafe, safe) => {
   if (Symbol.iterator in unsafe.prototype) {
     const dummy = new unsafe();
     let next; // We can reuse the same `next` method.
 
+    // First, wrap the known iterator-returning methods with SafeIterator.
+    ArrayPrototypeForEach(iteratorMethods, key => {
+      const desc = Reflect.getOwnPropertyDescriptor(unsafe.prototype, key);
+      if (desc && typeof desc.value === "function") {
+        const createIterator = uncurryThis(desc.value);
+        next ??= uncurryThis(createIterator(dummy).next);
+        const SafeIterator = createSafeIterator(createIterator, next);
+        desc.value = function () {
+          return new SafeIterator(this);
+        };
+        Reflect.defineProperty(safe.prototype, key, desc);
+      }
+    });
+
+    // Then, copy remaining built-in properties (skip already-defined and user-added keys).
     ArrayPrototypeForEach(Reflect.ownKeys(unsafe.prototype), key => {
       if (!Reflect.getOwnPropertyDescriptor(safe.prototype, key)) {
         const desc = Reflect.getOwnPropertyDescriptor(unsafe.prototype, key);
-        if (typeof desc.value === "function" && desc.value.length === 0) {
-          const called = desc.value.$call(dummy) || {};
-          if (Symbol.iterator in (typeof called === "object" ? called : {})) {
-            const createIterator = uncurryThis(desc.value);
-            next ??= uncurryThis(createIterator(dummy).next);
-            const SafeIterator = createSafeIterator(createIterator, next);
-            desc.value = function () {
-              return new SafeIterator(this);
-            };
-          }
+        if (desc) {
+          Reflect.defineProperty(safe.prototype, key, desc);
         }
-        Reflect.defineProperty(safe.prototype, key, desc);
       }
     });
   } else copyProps(unsafe.prototype, safe.prototype);

test/regression/issue/18890.test.ts

@@ -0,0 +1,62 @@
+import { expect, test } from "bun:test";
+import { bunEnv, bunExe } from "harness";
+
+// https://github.com/oven-sh/bun/issues/18890
+// Adding functions to Set/Map/WeakSet/WeakMap prototypes before import('fs')
+// should not cause those functions to be invoked by Bun's internal primordials.
+
+test("import('fs') does not invoke user-defined Set.prototype methods", async () => {
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "-e", "Set.prototype.hi = function () { throw 'hi' }; await import('fs')"],
+    env: bunEnv,
+    stderr: "pipe",
+  });
+
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+  expect(stderr).not.toContain("error: hi");
+  expect(exitCode).toBe(0);
+});
+
+test("import('fs') does not invoke user-defined Map.prototype methods", async () => {
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "-e", "Map.prototype.hi = function () { throw 'hi' }; await import('fs')"],
+    env: bunEnv,
+    stderr: "pipe",
+  });
+
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+  expect(stderr).not.toContain("error: hi");
+  expect(exitCode).toBe(0);
+});
+
+test("static import with prototype pollution does not throw", async () => {
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "-e", "Set.prototype.hi = function () { throw 'hi' }; require('fs'); import fs from 'fs'"],
+    env: bunEnv,
+    stderr: "pipe",
+  });
+
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+  expect(stderr).not.toContain("error: hi");
+  expect(exitCode).toBe(0);
+});
+
+test("fs module works correctly after prototype pollution", async () => {
+  await using proc = Bun.spawn({
+    cmd: [
+      bunExe(),
+      "-e",
+      "Set.prototype.hi = function () { throw 'hi' }; const fs = await import('fs'); console.log(typeof fs.readFileSync)",
+    ],
+    env: bunEnv,
+    stderr: "pipe",
+  });
+
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+  expect(stdout.trim()).toBe("function");
+  expect(exitCode).toBe(0);
+});