Skip to content

Commit 62229c0

Browse files
committed
vfs: prevent stack overflow in recursive readdir on circular symlinks
MemoryProvider#readdirSync with `recursive: true` follows symlinks to directories during traversal but did not bound the number of symlinks followed along a branch. A circular symlink therefore caused unbounded recursion in the internal walk() helper until the call stack was exhausted, crashing the process with `RangeError: Maximum call stack size exceeded`. Both the synchronous and promise-based variants were affected. The existing kMaxSymlinkDepth guard in #lookupEntry did not help, because walk() resolved each symlink target with a fresh depth of zero. Track the number of symlink hops along the current branch and stop recursing once it would exceed kMaxSymlinkDepth, mirroring the ELOOP guard in #lookupEntry and the behavior of the real filesystem, which follows directory symlinks until the OS symlink limit is reached. The entries themselves are still listed, so non-circular symlinks continue to be followed as before. Fixes: #64148 Signed-off-by: AkshatOP <hunterdevil0987@gmail.com>
1 parent 74567b2 commit 62229c0

2 files changed

Lines changed: 63 additions & 5 deletions

File tree

lib/internal/vfs/providers/memory.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,7 @@ class MemoryProvider extends VirtualProvider {
612612
#readdirRecursive(dirEntry, dirPath, withFileTypes) {
613613
const results = [];
614614

615-
const walk = (entry, currentPath, relativePath) => {
615+
const walk = (entry, currentPath, relativePath, symlinkDepth) => {
616616
this.#ensurePopulated(entry, currentPath);
617617

618618
for (const { 0: name, 1: childEntry } of entry.children) {
@@ -636,6 +636,7 @@ class MemoryProvider extends VirtualProvider {
636636

637637
// Follow symlinks to directories for recursive traversal
638638
let resolvedChild = childEntry;
639+
let childSymlinkDepth = symlinkDepth;
639640
if (childEntry.isSymbolicLink()) {
640641
const targetPath = this.#resolveSymlinkTarget(
641642
pathPosix.join(currentPath, name), childEntry.target,
@@ -644,15 +645,18 @@ class MemoryProvider extends VirtualProvider {
644645
if (result.entry) {
645646
resolvedChild = result.entry;
646647
}
648+
// Bound symlink hops to avoid unbounded recursion on cycles.
649+
childSymlinkDepth = symlinkDepth + 1;
647650
}
648-
if (resolvedChild.isDirectory()) {
651+
if (resolvedChild.isDirectory() &&
652+
childSymlinkDepth <= kMaxSymlinkDepth) {
649653
const childPath = pathPosix.join(currentPath, name);
650-
walk(resolvedChild, childPath, childRelative);
654+
walk(resolvedChild, childPath, childRelative, childSymlinkDepth);
651655
}
652656
}
653657
};
654658

655-
walk(dirEntry, dirPath, '');
659+
walk(dirEntry, dirPath, '', 0);
656660
return results;
657661
}
658662

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

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
// Recursive readdir must follow symlinks to directories.
55

6-
require('../common');
6+
const common = require('../common');
77
const assert = require('assert');
88
const vfs = require('node:vfs');
99

@@ -52,3 +52,57 @@ assert.ok(
5252
assert.ok(dirents.some((d) => d.name === 'sub' && d.isDirectory()));
5353
assert.ok(dirents.some((d) => d.name === 'lnk' && d.isSymbolicLink()));
5454
}
55+
56+
// Recursive readdir on circular symlinks must terminate, not overflow the
57+
// stack. Regression test for https://github.com/nodejs/node/issues/64148
58+
59+
// Self-referential symlink: /dir/loop -> /dir
60+
{
61+
const v = vfs.create();
62+
v.mkdirSync('/dir');
63+
v.symlinkSync('/dir', '/dir/loop');
64+
65+
const entries = v.readdirSync('/', { recursive: true });
66+
// Terminates, follows the symlink at least one level, stays bounded.
67+
assert.ok(entries.includes('dir'));
68+
assert.ok(entries.includes('dir/loop'));
69+
assert.ok(entries.includes('dir/loop/loop'));
70+
assert.ok(entries.length < 100, `unbounded result: ${entries.length}`);
71+
}
72+
73+
// Mutual circular chain: /a/link_to_b -> /b and /b/link_to_a -> /a
74+
{
75+
const v = vfs.create();
76+
v.mkdirSync('/a');
77+
v.mkdirSync('/b');
78+
v.symlinkSync('/a', '/b/link_to_a');
79+
v.symlinkSync('/b', '/a/link_to_b');
80+
81+
const entries = v.readdirSync('/', { recursive: true });
82+
assert.ok(entries.includes('a'));
83+
assert.ok(entries.includes('b'));
84+
assert.ok(entries.length < 200, `unbounded result: ${entries.length}`);
85+
}
86+
87+
// withFileTypes:true on a circular symlink must also terminate.
88+
{
89+
const v = vfs.create();
90+
v.mkdirSync('/dir');
91+
v.symlinkSync('/dir', '/dir/loop');
92+
93+
const dirents = v.readdirSync('/', { withFileTypes: true, recursive: true });
94+
assert.ok(dirents.some((d) => d.name === 'loop' && d.isSymbolicLink()));
95+
assert.ok(dirents.length < 100, `unbounded result: ${dirents.length}`);
96+
}
97+
98+
// Async promises.readdir variant must reject-free terminate as well.
99+
{
100+
const v = vfs.create();
101+
v.mkdirSync('/dir');
102+
v.symlinkSync('/dir', '/dir/loop');
103+
104+
v.promises.readdir('/', { recursive: true }).then(common.mustCall((entries) => {
105+
assert.ok(entries.includes('dir/loop'));
106+
assert.ok(entries.length < 100, `unbounded result: ${entries.length}`);
107+
}));
108+
}

0 commit comments

Comments
 (0)