lib,permission: require full read and write to symlink APIs

Refs: https://hackerone.com/reports/3417819
Signed-off-by: RafaelGSS <[email protected]>
PR-URL: nodejs-private/node-private#760
Reviewed-By: Matteo Collina <[email protected]>
CVE-ID: CVE-2025-55130

Author: RafaelGSS

lib/fs.js

@@ -59,7 +59,6 @@ const {
 } = constants;
 
 const pathModule = require('path');
-const { isAbsolute } = pathModule;
 const { isArrayBufferView } = require('internal/util/types');
 
 const binding = internalBinding('fs');
@@ -1811,18 +1810,12 @@ function symlink(target, path, type_, callback_) {
   const type = (typeof type_ === 'string' ? type_ : null);
   const callback = makeCallback(arguments[arguments.length - 1]);
 
-  if (permission.isEnabled()) {
-    // The permission model's security guarantees fall apart in the presence of
-    // relative symbolic links. Thus, we have to prevent their creation.
-    if (BufferIsBuffer(target)) {
-      if (!isAbsolute(BufferToString(target))) {
-        callback(new ERR_ACCESS_DENIED('relative symbolic link target'));
-        return;
-      }
-    } else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
-      callback(new ERR_ACCESS_DENIED('relative symbolic link target'));
-      return;
-    }
+  // Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass
+  // the permission model security guarantees. Thus, this API is disabled unless fs.read
+  // and fs.write permission has been given.
+  if (permission.isEnabled() && !permission.has('fs')) {
+    callback(new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.'));
+    return;
   }
 
   target = getValidatedPath(target, 'target');
@@ -1886,16 +1879,11 @@ function symlinkSync(target, path, type) {
     }
   }
 
-  if (permission.isEnabled()) {
-    // The permission model's security guarantees fall apart in the presence of
-    // relative symbolic links. Thus, we have to prevent their creation.
-    if (BufferIsBuffer(target)) {
-      if (!isAbsolute(BufferToString(target))) {
-        throw new ERR_ACCESS_DENIED('relative symbolic link target');
-      }
-    } else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
-      throw new ERR_ACCESS_DENIED('relative symbolic link target');
-    }
+  // Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass
+  // the permission model security guarantees. Thus, this API is disabled unless fs.read
+  // and fs.write permission has been given.
+  if (permission.isEnabled() && !permission.has('fs')) {
+    throw new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.');
   }
 
   target = getValidatedPath(target, 'target');

lib/internal/fs/promises.js

@@ -16,7 +16,6 @@ const {
   SafePromisePrototypeFinally,
   Symbol,
   Uint8Array,
-  uncurryThis,
 } = primordials;
 
 const { fs: constants } = internalBinding('constants');
@@ -30,8 +29,6 @@ const {
 
 const binding = internalBinding('fs');
 const { Buffer } = require('buffer');
-const { isBuffer: BufferIsBuffer } = Buffer;
-const BufferToString = uncurryThis(Buffer.prototype.toString);
 
 const {
   AbortError,
@@ -87,8 +84,6 @@ const {
   kValidateObjectAllowNullable,
 } = require('internal/validators');
 const pathModule = require('path');
-const { isAbsolute } = pathModule;
-const { toPathIfFileURL } = require('internal/url');
 const {
   getLazy,
   kEmptyObject,
@@ -991,16 +986,11 @@ async function symlink(target, path, type_) {
     }
   }
 
-  if (permission.isEnabled()) {
-    // The permission model's security guarantees fall apart in the presence of
-    // relative symbolic links. Thus, we have to prevent their creation.
-    if (BufferIsBuffer(target)) {
-      if (!isAbsolute(BufferToString(target))) {
-        throw new ERR_ACCESS_DENIED('relative symbolic link target');
-      }
-    } else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
-      throw new ERR_ACCESS_DENIED('relative symbolic link target');
-    }
+  // Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass
+  // the permission model security guarantees. Thus, this API is disabled unless fs.read
+  // and fs.write permission has been given.
+  if (permission.isEnabled() && !permission.has('fs')) {
+    throw new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.');
   }
 
   target = getValidatedPath(target, 'target');

test/fixtures/permission/fs-symlink-target-write.js

@@ -26,8 +26,7 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER;
     fs.symlinkSync(path.join(readOnlyFolder, 'file'), path.join(readWriteFolder, 'link-to-read-only'), 'file');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemWrite',
-    resource: path.toNamespacedPath(path.join(readOnlyFolder, 'file')),
+    message: 'fs.symlink API requires full fs.read and fs.write permissions.',
   }));
   assert.throws(() => {
     fs.linkSync(path.join(readOnlyFolder, 'file'), path.join(readWriteFolder, 'link-to-read-only'));
@@ -37,18 +36,6 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER;
     resource: path.toNamespacedPath(path.join(readOnlyFolder, 'file')),
   }));
 
-  // App will be able to symlink to a writeOnlyFolder
-  fs.symlink(path.join(readWriteFolder, 'file'), path.join(writeOnlyFolder, 'link-to-read-write'), 'file', (err) => {
-    assert.ifError(err);
-    // App will won't be able to read the symlink
-    fs.readFile(path.join(writeOnlyFolder, 'link-to-read-write'), common.expectsError({
-      code: 'ERR_ACCESS_DENIED',
-      permission: 'FileSystemRead',
-    }));
-
-    // App will be able to write to the symlink
-    fs.writeFile(path.join(writeOnlyFolder, 'link-to-read-write'), 'some content', common.mustSucceed());
-  });
   fs.link(path.join(readWriteFolder, 'file'), path.join(writeOnlyFolder, 'link-to-read-write2'), (err) => {
     assert.ifError(err);
     // App will won't be able to read the link
@@ -66,8 +53,7 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER;
     fs.symlinkSync(path.join(readWriteFolder, 'file'), path.join(readOnlyFolder, 'link-to-read-only'), 'file');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemWrite',
-    resource: path.toNamespacedPath(path.join(readOnlyFolder, 'link-to-read-only')),
+    message: 'fs.symlink API requires full fs.read and fs.write permissions.',
   }));
   assert.throws(() => {
     fs.linkSync(path.join(readWriteFolder, 'file'), path.join(readOnlyFolder, 'link-to-read-only'));

test/fixtures/permission/fs-symlink.js

@@ -54,7 +54,6 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK;
     fs.readFileSync(blockedFile);
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemRead',
   }));
   assert.throws(() => {
     fs.appendFileSync(blockedFile, 'data');
@@ -68,7 +67,6 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK;
     fs.symlinkSync(regularFile, blockedFolder + '/asdf', 'file');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemWrite',
   }));
   assert.throws(() => {
     fs.linkSync(regularFile, blockedFolder + '/asdf');
@@ -82,12 +80,26 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK;
     fs.symlinkSync(blockedFile, path.join(__dirname, '/asdf'), 'file');
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
-    permission: 'FileSystemRead',
   }));
   assert.throws(() => {
     fs.linkSync(blockedFile, path.join(__dirname, '/asdf'));
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemRead',
   }));
+}
+
+// fs.symlink API is blocked by default
+{
+  assert.throws(() => {
+    fs.symlinkSync(regularFile, regularFile);
+  }, common.expectsError({
+    message: 'fs.symlink API requires full fs.read and fs.write permissions.',
+    code: 'ERR_ACCESS_DENIED',
+  }));
+
+  fs.symlink(regularFile, regularFile, common.expectsError({
+    message: 'fs.symlink API requires full fs.read and fs.write permissions.',
+    code: 'ERR_ACCESS_DENIED',
+  }));
 }

test/parallel/test-permission-fs-symlink-relative.js

@@ -1,4 +1,4 @@
-// Flags: --permission --allow-fs-read=* --allow-fs-write=*
+// Flags: --permission --allow-fs-read=*
 'use strict';
 
 const common = require('../common');
@@ -15,7 +15,7 @@ const { symlinkSync, symlink, promises: { symlink: symlinkAsync } } = require('f
 
 const error = {
   code: 'ERR_ACCESS_DENIED',
-  message: /relative symbolic link target/,
+  message: /symlink API requires full fs\.read and fs\.write permissions/,
 };
 
 for (const targetString of ['a', './b/c', '../d', 'e/../f', 'C:drive-relative', 'ntfs:alternate']) {
@@ -32,14 +32,14 @@ for (const targetString of ['a', './b/c', '../d', 'e/../f', 'C:drive-relative',
   }
 }
 
-// Absolute should not throw
+// Absolute should throw too
 for (const targetString of [path.resolve('.')]) {
   for (const target of [targetString, Buffer.from(targetString)]) {
     for (const path of [__filename]) {
       symlink(target, path, common.mustCall((err) => {
         assert(err);
-        assert.strictEqual(err.code, 'EEXIST');
-        assert.match(err.message, /file already exists/);
+        assert.strictEqual(err.code, error.code);
+        assert.match(err.message, error.message);
       }));
     }
   }

test/parallel/test-permission-fs-symlink.js

@@ -27,15 +27,26 @@ const commonPathWildcard = path.join(__filename, '../../common*');
 const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md');
 const blockedFolder = tmpdir.resolve('subdirectory');
 const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md');
+const allowedFolder = tmpdir.resolve('allowed-folder');
+const traversalSymlink = path.join(allowedFolder, 'deep1', 'deep2', 'deep3', 'gotcha');
 
 {
   tmpdir.refresh();
   fs.mkdirSync(blockedFolder);
+  // Create deep directory structure for path traversal test
+  fs.mkdirSync(allowedFolder);
+  fs.writeFileSync(path.resolve(allowedFolder, '../protected-file.md'), 'protected');
+  fs.mkdirSync(path.join(allowedFolder, 'deep1'));
+  fs.mkdirSync(path.join(allowedFolder, 'deep1', 'deep2'));
+  fs.mkdirSync(path.join(allowedFolder, 'deep1', 'deep2', 'deep3'));
 }
 
 {
   // Symlink previously created
+  // fs.symlink API is allowed when full-read and full-write access
   fs.symlinkSync(blockedFile, symlinkFromBlockedFile);
+  // Create symlink for path traversal test - symlink points to parent directory
+  fs.symlinkSync(allowedFolder, traversalSymlink);
 }
 
 {
@@ -44,6 +55,7 @@ const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md');
     [
       '--permission',
       `--allow-fs-read=${file}`, `--allow-fs-read=${commonPathWildcard}`, `--allow-fs-read=${symlinkFromBlockedFile}`,
+      `--allow-fs-read=${allowedFolder}`,
       `--allow-fs-write=${symlinkFromBlockedFile}`,
       file,
     ],
@@ -53,6 +65,8 @@ const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md');
         BLOCKEDFOLDER: blockedFolder,
         BLOCKEDFILE: blockedFile,
         EXISTINGSYMLINK: symlinkFromBlockedFile,
+        TRAVERSALSYMLINK: traversalSymlink,
+        ALLOWEDFOLDER: allowedFolder,
       },
     }
   );