Limit VFS symlink depth (#6485)

jasnell · npaun · web-flow · commit d3557434d267 · 2026-04-02T12:20:50.000-07:00
Co-authored-by: Nicholas Paun &lt;npaun@cloudflare.com&gt;
diff --git a/src/workerd/io/bundle-fs-test.c++ b/src/workerd/io/bundle-fs-test.c++
@@ -151,5 +151,49 @@ KJ_TEST("Guarding against circular symlinks works") {
     KJ_EXPECT(vfs->resolve(env.js, "file:///c"_url) == kj::none);
   });
 }
+
+KJ_TEST("Guarding against deep non-circular symlink chains works") {
+  // Regression test: a deep chain of distinct symlinks (no cycle) must not
+  // cause a stack overflow. The recursion guard should reject the chain once
+  // the depth limit is exceeded.
+  TestFixture fixture;
+
+  fixture.runInIoContext([&](const TestFixture::Environment& env) {
+    auto vfs = newVirtualFileSystem(kj::heap<FsMap>(), getTmpDirectoryImpl());
+
+    auto maybeTemp = KJ_ASSERT_NONNULL(vfs->resolve(env.js, "file:///"_url));
+    auto& tempDir = maybeTemp.get<kj::Rc<Directory>>();
+
+    // Create a target file at the end of the chain.
+    auto targetResult = KJ_ASSERT_NONNULL(tempDir->tryOpen(
+        env.js, kj::Path({"target"}), Directory::OpenOptions{.createAs = FsType::FILE}));
+    auto& targetFile = targetResult.get<kj::Rc<File>>();
+    KJ_EXPECT(targetFile->write(env.js, 0, "hello"_kjb).get<uint32_t>() == 5);
+
+    // Build a chain of 300 distinct symlinks: link_0 -> link_1 -> ... -> link_299 -> target.
+    // This exceeds the depth limit (256) without forming a cycle.
+    constexpr int chainLen = 300;
+    for (int i = chainLen - 1; i >= 0; i--) {
+      kj::String target =
+          i == chainLen - 1 ? kj::str("file:///target") : kj::str("file:///link_", i + 1);
+      auto targetUrl = KJ_ASSERT_NONNULL(jsg::Url::tryParse(target.asPtr()));
+      KJ_EXPECT(tempDir->add(env.js, kj::str("link_", i),
+                    vfs->newSymbolicLink(env.js, targetUrl)) == kj::none);
+    }
+
+    // Resolving a link near the end of the chain should succeed (under the limit).
+    KJ_ASSERT_NONNULL(vfs->resolve(env.js, "file:///link_299"_url));
+
+    // Resolving from the start of the chain must fail with SYMLINK_DEPTH_EXCEEDED,
+    // not crash with a stack overflow.
+    auto resolved = KJ_ASSERT_NONNULL(vfs->resolve(env.js, "file:///link_0"_url));
+    KJ_EXPECT(resolved.get<workerd::FsError>() == workerd::FsError::SYMLINK_DEPTH_EXCEEDED);
+
+    // stat should also fail gracefully.
+    auto resolvedStat = KJ_ASSERT_NONNULL(vfs->resolveStat(env.js, "file:///link_0"_url));
+    KJ_EXPECT(resolvedStat.get<workerd::FsError>() == workerd::FsError::SYMLINK_DEPTH_EXCEEDED);
+  });
+}
+
 }  // namespace
 }  // namespace workerd
diff --git a/src/workerd/io/worker-fs.c++ b/src/workerd/io/worker-fs.c++
@@ -40,6 +40,9 @@ bool setCurrentWorkingDirectory(kj::Path newCwd) {
 }
 
 namespace {
+// Maximum distinct symlinks that can be traversed during a single resolution.
+static constexpr size_t kMaxSymlinkDepth = 256;
+
 // The SymbolicLinkRecursionGuardScope is used on-stack to guard against
 // circular symbolic links. As soon as a cycle is detected, it throws.
 // Since resolution is always synchronous, we can use thread-local to
@@ -1439,6 +1442,9 @@ kj::Maybe<FsError> SymbolicLinkRecursionGuardScope::checkSeen(SymbolicLink* link
     return FsError::SYMLINK_DEPTH_EXCEEDED;
   }
   guard.linksSeen.insert(link);
+  if (guard.linksSeen.size() > kMaxSymlinkDepth) {
+    return FsError::SYMLINK_DEPTH_EXCEEDED;
+  }
   return kj::none;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -40,6 +40,9 @@ bool setCurrentWorkingDirectory(kj::Path newCwd) {`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`namespace {`
	`43`	`+// Maximum distinct symlinks that can be traversed during a single resolution.`
	`44`	`+static constexpr size_t kMaxSymlinkDepth = 256;`
	`45`	`+`
`43`	`46`	`// The SymbolicLinkRecursionGuardScope is used on-stack to guard against`
`44`	`47`	`// circular symbolic links. As soon as a cycle is detected, it throws.`
`45`	`48`	`// Since resolution is always synchronous, we can use thread-local to`
`@@ -1439,6 +1442,9 @@ kj::Maybe<FsError> SymbolicLinkRecursionGuardScope::checkSeen(SymbolicLink* link`
`1439`	`1442`	`return FsError::SYMLINK_DEPTH_EXCEEDED;`
`1440`	`1443`	`}`
`1441`	`1444`	`guard.linksSeen.insert(link);`
	`1445`	`+ if (guard.linksSeen.size() > kMaxSymlinkDepth) {`
	`1446`	`+ return FsError::SYMLINK_DEPTH_EXCEEDED;`
	`1447`	`+ }`
`1442`	`1448`	`return kj::none;`
`1443`	`1449`	`}`
`1444`	`1450`