Symbolic link handling / Symlink following leading to unintended deletion or path traversal

// PoC to illustrate the potential symlink-following issue that this fix addresses (requires a Node version before 25.9.0). // WARNING: Do not run against real system files. This uses temporary files under /tmp. const fs = require('fs'); const os = require('os'); const path = require('path'); // Create a sandbox-like environment in /tmp const base = fs.mkdtempSync(path.join(os.tmpdir(), 'node-vuln-')); const parent = path.dirname(base); // Create a target outside the sandbox const outsideTarget = path.join(parent, 'outside-target.txt'); fs.writeFileSync(outsideTarget, 'sensitive data', 'utf8'); // Create a symlink inside the sandbox pointing to the outside target const symlinkInSandbox = path.join(base, 'link-to-outside'); fs.symlinkSync(outsideTarget, symlinkInSandbox); console.log('Symlink created:', symlinkInSandbox, '->', outsideTarget); // Attempt to remove the symlink using rmSync // In vulnerable behavior (pre-fix), if rmSync followed the symlink, it could affect the target outside the sandbox. // The fixed behavior uses symlink_status to avoid following the symlink. try { fs.rmSync(symlinkInSandbox); console.log('rmSync finished on symlink inside sandbox.'); } catch (e) { console.error('rmSync error:', e); } // Check whether the outside target still exists (vulnerability would show the target being affected) const targetExists = fs.existsSync(outsideTarget); console.log('Outside target exists after rmSync:', targetExists ? 'YES' : 'NO'); // Cleanup: remove any remnants and the outside target try { if (fs.existsSync(symlinkInSandbox) || true) fs.rmSync(base, { recursive: true, force: true }); } catch (e) {} try { if (fs.existsSync(outsideTarget)) fs.rmSync(outsideTarget, { force: true }); } catch (e) {}

diff --git a/src/node_file.cc b/src/node_file.cc index d925605feba8a6..3a9f0fdd0bbc01 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1636,7 +1636,7 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) { env, permission::PermissionScope::kFileSystemWrite, path.ToStringView()); auto file_path = std::filesystem::path(path.ToStringView()); std::error_code error; - auto file_status = std::filesystem::status(file_path, error); + auto file_status = std::filesystem::symlink_status(file_path, error); if (file_status.type() == std::filesystem::file_type::not_found) { return; diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index 8169122f1f79e1..4104c2e948da0e 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -165,10 +165,13 @@ function removeAsync(dir) { // Should delete an invalid symlink const invalidLink = tmpdir.resolve('invalid-link-async'); fs.symlinkSync('definitely-does-not-exist-async', invalidLink); + assert.ok(fs.lstatSync(invalidLink).isSymbolicLink()); + // `existsSync()` follows symlinks, so this confirms the target does not exist. + assert.strictEqual(fs.existsSync(invalidLink), false); fs.rm(invalidLink, common.mustNotMutateObjectDeep({ recursive: true }), common.mustCall((err) => { try { assert.strictEqual(err, null); - assert.strictEqual(fs.existsSync(invalidLink), false); + assert.throws(() => fs.lstatSync(invalidLink), { code: 'ENOENT' }); } finally { fs.rmSync(invalidLink, common.mustNotMutateObjectDeep({ force: true })); } @@ -247,11 +250,14 @@ if (isGitPresent) { } // Should delete an invalid symlink + // Refs: https://github.com/nodejs/node/issues/61020 const invalidLink = tmpdir.resolve('invalid-link'); fs.symlinkSync('definitely-does-not-exist', invalidLink); + assert.ok(fs.lstatSync(invalidLink).isSymbolicLink()); + assert.strictEqual(fs.existsSync(invalidLink), false); try { fs.rmSync(invalidLink); - assert.strictEqual(fs.existsSync(invalidLink), false); + assert.throws(() => fs.lstatSync(invalidLink), { code: 'ENOENT' }); } finally { fs.rmSync(invalidLink, common.mustNotMutateObjectDeep({ force: true })); } @@ -355,9 +361,11 @@ if (isGitPresent) { // Should delete an invalid symlink const invalidLink = tmpdir.resolve('invalid-link-prom'); fs.symlinkSync('definitely-does-not-exist-prom', invalidLink); + assert.ok(fs.lstatSync(invalidLink).isSymbolicLink()); + assert.strictEqual(fs.existsSync(invalidLink), false); try { await fs.promises.rm(invalidLink); - assert.strictEqual(fs.existsSync(invalidLink), false); + assert.throws(() => fs.lstatSync(invalidLink), { code: 'ENOENT' }); } finally { fs.rmSync(invalidLink, common.mustNotMutateObjectDeep({ force: true })); }