Skip to content

Commit 2e4e1be

Browse files
committed
vfs: prevent stack overflow in recursive readdir on circular symlinks
MemoryProvider#readdirSync with `recursive: true` follows symlinks to directories during traversal. The traversal was recursive and unbounded, so a circular symlink 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. Rewrite the traversal to be iterative, using an explicit queue instead of recursion so deeply nested trees can no longer exhaust the call stack. Each queued directory carries the number of symlink hops taken to reach it; a directory reached by following a symlink is not enqueued once that count would exceed kMaxSymlinkDepth, mirroring the ELOOP guard in #lookupEntry and the real filesystem, which follows directory symlinks until the OS symlink limit is reached. Entries 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 2e4e1be

2 files changed

Lines changed: 81 additions & 8 deletions

File tree

lib/internal/vfs/providers/memory.js

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

615-
const walk = (entry, currentPath, relativePath) => {
615+
// Traverse iteratively using an explicit queue instead of recursion, so a
616+
// deeply nested tree cannot exhaust the call stack. Each queued directory
617+
// also carries the number of symlink hops taken to reach it; following a
618+
// symlink stops once that count would exceed kMaxSymlinkDepth, so circular
619+
// symlinks terminate just as they do on the real filesystem (ELOOP).
620+
const queue = [{
621+
entry: dirEntry,
622+
path: dirPath,
623+
relativePath: '',
624+
symlinkDepth: 0,
625+
}];
626+
627+
for (let i = 0; i < queue.length; i++) {
628+
const { entry, path: currentPath, relativePath, symlinkDepth } = queue[i];
616629
this.#ensurePopulated(entry, currentPath);
617630

618631
for (const { 0: name, 1: childEntry } of entry.children) {
@@ -634,8 +647,9 @@ class MemoryProvider extends VirtualProvider {
634647
ArrayPrototypePush(results, childRelative);
635648
}
636649

637-
// Follow symlinks to directories for recursive traversal
650+
// Follow symlinks to directories for recursive traversal.
638651
let resolvedChild = childEntry;
652+
let childSymlinkDepth = symlinkDepth;
639653
if (childEntry.isSymbolicLink()) {
640654
const targetPath = this.#resolveSymlinkTarget(
641655
pathPosix.join(currentPath, name), childEntry.target,
@@ -644,15 +658,20 @@ class MemoryProvider extends VirtualProvider {
644658
if (result.entry) {
645659
resolvedChild = result.entry;
646660
}
661+
childSymlinkDepth = symlinkDepth + 1;
647662
}
648-
if (resolvedChild.isDirectory()) {
649-
const childPath = pathPosix.join(currentPath, name);
650-
walk(resolvedChild, childPath, childRelative);
663+
if (resolvedChild.isDirectory() &&
664+
childSymlinkDepth <= kMaxSymlinkDepth) {
665+
ArrayPrototypePush(queue, {
666+
entry: resolvedChild,
667+
path: pathPosix.join(currentPath, name),
668+
relativePath: childRelative,
669+
symlinkDepth: childSymlinkDepth,
670+
});
651671
}
652672
}
653-
};
673+
}
654674

655-
walk(dirEntry, dirPath, '');
656675
return results;
657676
}
658677

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)