From 7a2f106263ee7787660e7ec4c60c4446ebfb8239 Mon Sep 17 00:00:00 2001 From: fg0x0 Date: Sat, 9 May 2026 13:52:04 +0800 Subject: [PATCH] permission: check symlink target in cpSync fs.cpSync with recursive:true calls create_symlink() and copy_symlink() without checking if the symlink target is within the allowed permission paths. fs.symlinkSync already validates symlink targets against the permission model (added as the fix for CVE-2025-55130 at src/node_file.cc:1353-1357). The same check was missing in CpSyncCopyDir for both the standard symlink copy path and the verbatimSymlinks code path. Add permission checks for both kFileSystemRead and kFileSystemWrite on the resolved symlink target before create_symlink, create_directory_symlink, and copy_symlink calls in CpSyncCopyDir. Fixes: https://github.com/nodejs/node/issues/63179 Refs: CVE-2025-55130 Signed-off-by: fg0x0 --- src/node_file.cc | 52 ++++++++++- .../parallel/test-fs-cp-permission-symlink.js | 93 +++++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-fs-cp-permission-symlink.js diff --git a/src/node_file.cc b/src/node_file.cc index d93f213202ec43..87907136986b87 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -88,7 +88,7 @@ using v8::Undefined; using v8::Value; #ifndef S_ISDIR -#define S_ISDIR(mode) (((mode)&S_IFMT) == S_IFDIR) +#define S_ISDIR(mode) (((mode) & S_IFMT) == S_IFDIR) #endif #ifdef __POSIX__ @@ -3752,6 +3752,30 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { if (dir_entry.is_symlink()) { if (verbatim_symlinks) { + // Permission check for verbatimSymlinks path (incomplete + // CVE-2025-55130 fix) + if (env->permission()->enabled()) { + auto verb_target = std::filesystem::read_symlink(src, error); + if (error) break; + auto verb_target_abs = std::filesystem::weakly_canonical( + std::filesystem::absolute(src.parent_path() / verb_target)); + auto verb_str = verb_target_abs.string(); + auto verb_view = std::string_view(verb_str); + if (!env->permission()->is_granted( + env, + permission::PermissionScope::kFileSystemRead, + verb_view) || + !env->permission()->is_granted( + env, + permission::PermissionScope::kFileSystemWrite, + verb_view)) { + return THROW_ERR_ACCESS_DENIED( + env, + "Access to symlink target '%s' denied", + verb_str.c_str()); + } + } + std::filesystem::copy_symlink( dir_entry.path(), dest_file_path, error); if (error) { @@ -3818,6 +3842,32 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { } auto symlink_target_absolute = std::filesystem::weakly_canonical( std::filesystem::absolute(src / symlink_target)); + + // Permission check for symlink target (incomplete CVE-2025-55130 fix) + // Ensure the symlink target is within allowed permission paths + if (env->permission()->enabled()) { + auto target_str = symlink_target_absolute.string(); + auto target_view = std::string_view(target_str); + if (!env->permission()->is_granted( + env, + permission::PermissionScope::kFileSystemRead, + target_view)) { + return THROW_ERR_ACCESS_DENIED( + env, + "Access to symlink target '%s' denied", + target_str.c_str()); + } + if (!env->permission()->is_granted( + env, + permission::PermissionScope::kFileSystemWrite, + target_view)) { + return THROW_ERR_ACCESS_DENIED( + env, + "Access to symlink target '%s' denied", + target_str.c_str()); + } + } + if (dir_entry.is_directory()) { std::filesystem::create_directory_symlink( symlink_target_absolute, dest_file_path, error); diff --git a/test/parallel/test-fs-cp-permission-symlink.js b/test/parallel/test-fs-cp-permission-symlink.js new file mode 100644 index 00000000000000..c747a88d1d6528 --- /dev/null +++ b/test/parallel/test-fs-cp-permission-symlink.js @@ -0,0 +1,93 @@ +// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) common.skip('missing crypto'); + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const { execFileSync } = require('child_process'); + +// This test verifies that fs.cpSync checks symlink target permissions +// when copying directories containing symlinks. +// Regression test for incomplete CVE-2025-55130 fix. + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +const allowedDir = path.join(tmpdir.path, 'allowed'); +const deniedDir = path.join(tmpdir.path, 'denied'); +const srcDir = path.join(allowedDir, 'src'); +const destDir = path.join(allowedDir, 'dest'); +const secretFile = path.join(deniedDir, 'secret.txt'); + +// Setup directories +fs.mkdirSync(srcDir, { recursive: true }); +fs.mkdirSync(destDir, { recursive: true }); +fs.mkdirSync(deniedDir, { recursive: true }); +fs.writeFileSync(secretFile, 'SECRET_DATA'); + +// Create symlink pointing outside allowed path +fs.symlinkSync(secretFile, path.join(srcDir, 'link')); + +// Run with restricted permissions — only allowedDir is permitted +const result = execFileSync(process.execPath, [ + '--experimental-permission', + `--allow-fs-read=${allowedDir}`, + `--allow-fs-write=${allowedDir}`, + '--allow-fs-read=/usr', + '--allow-fs-read=/lib', + '-e', + ` + const fs = require('node:fs'); + try { + fs.cpSync('${srcDir}/', '${destDir}/', { recursive: true }); + console.log('FAIL'); + } catch(e) { + console.log(e.code); + } + `, +], { encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'] }).trim(); + +// cpSync should throw ERR_ACCESS_DENIED because symlink target +// (/tmp/.../denied/secret.txt) is outside allowed paths +assert.strictEqual( + result, + 'ERR_ACCESS_DENIED', + `Expected ERR_ACCESS_DENIED but got: ${result}` +); + +// Also test verbatimSymlinks path +const destDir2 = path.join(allowedDir, 'dest2'); +fs.mkdirSync(destDir2, { recursive: true }); + +const result2 = execFileSync(process.execPath, [ + '--experimental-permission', + `--allow-fs-read=${allowedDir}`, + `--allow-fs-write=${allowedDir}`, + '--allow-fs-read=/usr', + '--allow-fs-read=/lib', + '-e', + ` + const fs = require('node:fs'); + try { + fs.cpSync('${srcDir}/', '${destDir2}/', { + recursive: true, + verbatimSymlinks: true + }); + console.log('FAIL'); + } catch(e) { + console.log(e.code); + } + `, +], { encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'] }).trim(); + +assert.strictEqual( + result2, + 'ERR_ACCESS_DENIED', + `verbatimSymlinks: Expected ERR_ACCESS_DENIED but got: ${result2}` +); + +console.log('All permission checks passed.');