vfs: avoid recursive readdir symlink cycles

Track the active MemoryProvider recursive readdir traversal path so
circular symlinks to directories stop recursing while still listing
the symlink entries.

Add sync, promise, and withFileTypes coverage.

Fixes: https://github.com/nodejs/node/issues/64148
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/64168
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>

lib/internal/vfs/providers/memory.js

@@ -5,6 +5,7 @@ const {
   ArrayPrototypePush,
   DateNow,
   SafeMap,
+  SafeSet,
   StringPrototypeReplaceAll,
   Symbol,
 } = primordials;
@@ -611,44 +612,55 @@ class MemoryProvider extends VirtualProvider {
    */
   #readdirRecursive(dirEntry, dirPath, withFileTypes) {
     const results = [];
+    const active = new SafeSet();
 
     const walk = (entry, currentPath, relativePath) => {
-      this.#ensurePopulated(entry, currentPath);
+      if (active.has(entry)) {
+        return;
+      }
 
-      for (const { 0: name, 1: childEntry } of entry.children) {
-        const childRelative = relativePath ?
-          relativePath + '/' + name : name;
+      active.add(entry);
+      try {
+        this.#ensurePopulated(entry, currentPath);
 
-        if (withFileTypes) {
-          let type;
-          if (childEntry.isSymbolicLink()) {
-            type = UV_DIRENT_LINK;
-          } else if (childEntry.isDirectory()) {
-            type = UV_DIRENT_DIR;
+        for (const { 0: name, 1: childEntry } of entry.children) {
+          const childRelative = relativePath ?
+            relativePath + '/' + name : name;
+
+          if (withFileTypes) {
+            let type;
+            if (childEntry.isSymbolicLink()) {
+              type = UV_DIRENT_LINK;
+            } else if (childEntry.isDirectory()) {
+              type = UV_DIRENT_DIR;
+            } else {
+              type = UV_DIRENT_FILE;
+            }
+            ArrayPrototypePush(results,
+                               new Dirent(childRelative, type, dirPath));
           } else {
-            type = UV_DIRENT_FILE;
+            ArrayPrototypePush(results, childRelative);
           }
-          ArrayPrototypePush(results,
-                             new Dirent(childRelative, type, dirPath));
-        } else {
-          ArrayPrototypePush(results, childRelative);
-        }
 
-        // Follow symlinks to directories for recursive traversal
-        let resolvedChild = childEntry;
-        if (childEntry.isSymbolicLink()) {
-          const targetPath = this.#resolveSymlinkTarget(
-            pathPosix.join(currentPath, name), childEntry.target,
-          );
-          const result = this.#lookupEntry(targetPath, true, 0);
-          if (result.entry) {
-            resolvedChild = result.entry;
+          // Follow symlinks to directories for recursive traversal.
+          // Track the active traversal path to avoid symlink cycles.
+          let resolvedChild = childEntry;
+          if (childEntry.isSymbolicLink()) {
+            const targetPath = this.#resolveSymlinkTarget(
+              pathPosix.join(currentPath, name), childEntry.target,
+            );
+            const result = this.#lookupEntry(targetPath, true, 0);
+            if (result.entry) {
+              resolvedChild = result.entry;
+            }
+          }
+          if (resolvedChild.isDirectory()) {
+            const childPath = pathPosix.join(currentPath, name);
+            walk(resolvedChild, childPath, childRelative);
           }
         }
-        if (resolvedChild.isDirectory()) {
-          const childPath = pathPosix.join(currentPath, name);
-          walk(resolvedChild, childPath, childRelative);
-        }
+      } finally {
+        active.delete(entry);
       }
     };
 

test/parallel/test-vfs-readdir-symlink-recursive.js

@@ -3,7 +3,7 @@
 
 // Recursive readdir must follow symlinks to directories.
 
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const vfs = require('node:vfs');
 
@@ -21,6 +21,59 @@ assert.ok(
   `Expected 'symdir/nested.txt' in entries: ${entries}`,
 );
 
+// Recursive readdir avoids following symlink cycles indefinitely.
+{
+  const v = vfs.create();
+  v.mkdirSync('/dir');
+  v.writeFileSync('/dir/nested.txt', 'nested');
+  v.symlinkSync('/dir', '/dir/loop');
+
+  assert.deepStrictEqual(v.readdirSync('/', { recursive: true }).sort(), [
+    'dir',
+    'dir/loop',
+    'dir/nested.txt',
+  ]);
+
+  const dirents = v.readdirSync('/', { recursive: true, withFileTypes: true });
+  assert.strictEqual(dirents.length, 3);
+  assert.ok(dirents.some((dirent) =>
+    dirent.name === 'loop' &&
+    dirent.parentPath === '/dir' &&
+    dirent.isSymbolicLink()));
+}
+
+// Recursive readdir avoids cycles through multiple symlinks.
+{
+  const v = vfs.create();
+  v.mkdirSync('/a');
+  v.mkdirSync('/b');
+  v.symlinkSync('/b', '/a/link_to_b');
+  v.symlinkSync('/a', '/b/link_to_a');
+
+  assert.deepStrictEqual(v.readdirSync('/', { recursive: true }).sort(), [
+    'a',
+    'a/link_to_b',
+    'a/link_to_b/link_to_a',
+    'b',
+    'b/link_to_a',
+    'b/link_to_a/link_to_b',
+  ]);
+}
+
+(async () => {
+  const v = vfs.create();
+  v.mkdirSync('/dir');
+  v.writeFileSync('/dir/nested.txt', 'nested');
+  v.symlinkSync('/dir', '/dir/loop');
+
+  const entries = await v.promises.readdir('/', { recursive: true });
+  assert.deepStrictEqual(entries.sort(), [
+    'dir',
+    'dir/loop',
+    'dir/nested.txt',
+  ]);
+})().then(common.mustCall());
+
 // Recursive readdir with withFileTypes:true returns Dirent objects whose
 // parentPath reflects the actual location of the entry (not the entry's
 // stringified relative path).