[Security][High] isPathSafe blocks legitimate filenames & does not prevent symlink escape

[JkarVN]

Summary

Two design flaws in isPathSafe() (fs.cpp:153–169):

1. Over-broad .. rejection — any two consecutive dots in the path are rejected, even in legitimate filenames like file..txt, v1..0-release, or a...b.
2. No symlink escape prevention — isPathSafe checks literal path strings but never resolves symlinks, so a symlink inside the sandbox pointing outside completely bypasses the check.

Problem 1: False positives on legitimate filenames

// fs.cpp:158-160
for (const char* p = p_path; *p; ++p) {
    if (*p == '.' && *(p + 1) == '.') return false;
}

PoC

const fs = require('fs');
fs.writeFileSync("file..txt", "hello");  // ❌ SecurityError — legitimate filename!
fs.writeFileSync("a...b", "test");       // ❌ SecurityError

Problem 2: Symlink escape

// Inside sandbox:
require('fs').symlinkSync("/etc", "etc_link");
// Later:
require('fs').readFileSync("etc_link/passwd");  // ✅ passes isPathSafe, reads /etc/passwd

isPathSafe("etc_link/passwd") sees no .., no leading /, no C: → passes. But the resolved canonical path escapes the sandbox.

Problem 3: All absolute paths are blocked

if (p_path[0] == '/' || p_path[0] == '\') return false;
if (p_path[0] != '\0' && p_path[1] == ':') return false;

This makes the module unusable for any scenario requiring absolute paths (which Node.js fs normally supports). The sandbox restriction should be applied after path resolution, not by blocking the syntax.

Suggested Fix: Replace with root-jail model

Instead of blacklisting path syntax, resolve the canonical path and check it stays within an allowed root:

static inline bool isPathSafe(const char* p_path) {
    if (!p_path || p_path[0] == '\0') return false;
    
    // Reject NULL bytes (prevent C-string truncation tricks)
    size_t len = strlen(p_path);
    if (memchr(p_path, '\0', len)) return false;  // redundant with strlen, but defensive
    
    // Resolve canonical path
    std::error_code ec;
    fs::path resolved = fs::canonical(p_path, ec);
    if (ec) {
        // Path doesn't exist yet — try lexical normalization for write targets
        resolved = fs::path(p_path).lexically_normal();
        // If still relative, resolve against CWD
        if (resolved.is_relative()) {
            resolved = fs::current_path(ec) / resolved;
            resolved = resolved.lexically_normal();
        }
    }
    
    // Must stay within the sandbox root
    static const auto root = fs::current_path();  // or configured root
    auto root_str = root.string();
    auto resolved_str = resolved.string();
    
    // resolved must start with root prefix
    if (resolved_str.length() < root_str.length()) return false;
    if (resolved_str.compare(0, root_str.length(), root_str) != 0) return false;
    
    return true;
}

This approach:

• ✅ Allows .. in legitimate filenames (file..txt)
• ✅ Allows absolute paths that stay within root
• ✅ Prevents symlink escape (canonical resolves symlinks)
• ✅ Still prevents traversal out of the sandbox