|
| 1 | +From b9f628994a4c17f42fa9fc96337db89ccaeef0f9 Mon Sep 17 00:00:00 2001 |
| 2 | +From: RafaelGSS <rafael.nunu@hotmail.com> |
| 3 | +Date: Mon, 10 Nov 2025 19:27:51 -0300 |
| 4 | +Subject: [PATCH] lib,permission: require full read and write to symlink APIs |
| 5 | + |
| 6 | +Refs: https://hackerone.com/reports/3417819 |
| 7 | +PR-URL: https://github.com/nodejs-private/node-private/pull/760 |
| 8 | +Reviewed-By: Matteo Collina <matteo.collina@gmail.com> |
| 9 | +CVE-ID: CVE-2025-55130 |
| 10 | +Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> |
| 11 | +Signed-off-by: Azure Linux Security Servicing Account <azurelinux-security@microsoft.com> |
| 12 | +Upstream-reference: https://github.com/nodejs/node/commit/494f62dc23.patch |
| 13 | +--- |
| 14 | + lib/fs.js | 34 ++++++------------- |
| 15 | + lib/internal/fs/promises.js | 20 +++-------- |
| 16 | + .../permission/fs-symlink-target-write.js | 18 ++-------- |
| 17 | + test/fixtures/permission/fs-symlink.js | 18 ++++++++-- |
| 18 | + .../test-permission-fs-symlink-relative.js | 10 +++--- |
| 19 | + test/parallel/test-permission-fs-symlink.js | 14 ++++++++ |
| 20 | + 6 files changed, 52 insertions(+), 62 deletions(-) |
| 21 | + |
| 22 | +diff --git a/lib/fs.js b/lib/fs.js |
| 23 | +index fd339900..85f8301a 100644 |
| 24 | +--- a/lib/fs.js |
| 25 | ++++ b/lib/fs.js |
| 26 | +@@ -59,7 +59,6 @@ const { |
| 27 | + } = constants; |
| 28 | + |
| 29 | + const pathModule = require('path'); |
| 30 | +-const { isAbsolute } = pathModule; |
| 31 | + const { isArrayBufferView } = require('internal/util/types'); |
| 32 | + |
| 33 | + const binding = internalBinding('fs'); |
| 34 | +@@ -1736,18 +1735,12 @@ function symlink(target, path, type_, callback_) { |
| 35 | + const type = (typeof type_ === 'string' ? type_ : null); |
| 36 | + const callback = makeCallback(arguments[arguments.length - 1]); |
| 37 | + |
| 38 | +- if (permission.isEnabled()) { |
| 39 | +- // The permission model's security guarantees fall apart in the presence of |
| 40 | +- // relative symbolic links. Thus, we have to prevent their creation. |
| 41 | +- if (BufferIsBuffer(target)) { |
| 42 | +- if (!isAbsolute(BufferToString(target))) { |
| 43 | +- callback(new ERR_ACCESS_DENIED('relative symbolic link target')); |
| 44 | +- return; |
| 45 | +- } |
| 46 | +- } else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) { |
| 47 | +- callback(new ERR_ACCESS_DENIED('relative symbolic link target')); |
| 48 | +- return; |
| 49 | +- } |
| 50 | ++ // Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass |
| 51 | ++ // the permission model security guarantees. Thus, this API is disabled unless fs.read |
| 52 | ++ // and fs.write permission has been given. |
| 53 | ++ if (permission.isEnabled() && !permission.has('fs')) { |
| 54 | ++ callback(new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.')); |
| 55 | ++ return; |
| 56 | + } |
| 57 | + |
| 58 | + target = getValidatedPath(target, 'target'); |
| 59 | +@@ -1807,16 +1800,11 @@ function symlinkSync(target, path, type) { |
| 60 | + } |
| 61 | + } |
| 62 | + |
| 63 | +- if (permission.isEnabled()) { |
| 64 | +- // The permission model's security guarantees fall apart in the presence of |
| 65 | +- // relative symbolic links. Thus, we have to prevent their creation. |
| 66 | +- if (BufferIsBuffer(target)) { |
| 67 | +- if (!isAbsolute(BufferToString(target))) { |
| 68 | +- throw new ERR_ACCESS_DENIED('relative symbolic link target'); |
| 69 | +- } |
| 70 | +- } else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) { |
| 71 | +- throw new ERR_ACCESS_DENIED('relative symbolic link target'); |
| 72 | +- } |
| 73 | ++ // Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass |
| 74 | ++ // the permission model security guarantees. Thus, this API is disabled unless fs.read |
| 75 | ++ // and fs.write permission has been given. |
| 76 | ++ if (permission.isEnabled() && !permission.has('fs')) { |
| 77 | ++ throw new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.'); |
| 78 | + } |
| 79 | + |
| 80 | + target = getValidatedPath(target, 'target'); |
| 81 | +diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js |
| 82 | +index 1544c34e..5584b2e3 100644 |
| 83 | +--- a/lib/internal/fs/promises.js |
| 84 | ++++ b/lib/internal/fs/promises.js |
| 85 | +@@ -18,7 +18,6 @@ const { |
| 86 | + SymbolAsyncDispose, |
| 87 | + Uint8Array, |
| 88 | + FunctionPrototypeBind, |
| 89 | +- uncurryThis, |
| 90 | + } = primordials; |
| 91 | + |
| 92 | + const { fs: constants } = internalBinding('constants'); |
| 93 | +@@ -32,8 +31,6 @@ const { |
| 94 | + |
| 95 | + const binding = internalBinding('fs'); |
| 96 | + const { Buffer } = require('buffer'); |
| 97 | +-const { isBuffer: BufferIsBuffer } = Buffer; |
| 98 | +-const BufferToString = uncurryThis(Buffer.prototype.toString); |
| 99 | + |
| 100 | + const { |
| 101 | + codes: { |
| 102 | +@@ -89,8 +86,6 @@ const { |
| 103 | + kValidateObjectAllowNullable, |
| 104 | + } = require('internal/validators'); |
| 105 | + const pathModule = require('path'); |
| 106 | +-const { isAbsolute } = pathModule; |
| 107 | +-const { toPathIfFileURL } = require('internal/url'); |
| 108 | + const { |
| 109 | + kEmptyObject, |
| 110 | + lazyDOMException, |
| 111 | +@@ -980,16 +975,11 @@ async function symlink(target, path, type_) { |
| 112 | + } |
| 113 | + } |
| 114 | + |
| 115 | +- if (permission.isEnabled()) { |
| 116 | +- // The permission model's security guarantees fall apart in the presence of |
| 117 | +- // relative symbolic links. Thus, we have to prevent their creation. |
| 118 | +- if (BufferIsBuffer(target)) { |
| 119 | +- if (!isAbsolute(BufferToString(target))) { |
| 120 | +- throw new ERR_ACCESS_DENIED('relative symbolic link target'); |
| 121 | +- } |
| 122 | +- } else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) { |
| 123 | +- throw new ERR_ACCESS_DENIED('relative symbolic link target'); |
| 124 | +- } |
| 125 | ++ // Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass |
| 126 | ++ // the permission model security guarantees. Thus, this API is disabled unless fs.read |
| 127 | ++ // and fs.write permission has been given. |
| 128 | ++ if (permission.isEnabled() && !permission.has('fs')) { |
| 129 | ++ throw new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.'); |
| 130 | + } |
| 131 | + |
| 132 | + target = getValidatedPath(target, 'target'); |
| 133 | +diff --git a/test/fixtures/permission/fs-symlink-target-write.js b/test/fixtures/permission/fs-symlink-target-write.js |
| 134 | +index c17d674d..6e07bfa8 100644 |
| 135 | +--- a/test/fixtures/permission/fs-symlink-target-write.js |
| 136 | ++++ b/test/fixtures/permission/fs-symlink-target-write.js |
| 137 | +@@ -26,8 +26,7 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER; |
| 138 | + fs.symlinkSync(path.join(readOnlyFolder, 'file'), path.join(readWriteFolder, 'link-to-read-only'), 'file'); |
| 139 | + }, common.expectsError({ |
| 140 | + code: 'ERR_ACCESS_DENIED', |
| 141 | +- permission: 'FileSystemWrite', |
| 142 | +- resource: path.toNamespacedPath(path.join(readOnlyFolder, 'file')), |
| 143 | ++ message: 'fs.symlink API requires full fs.read and fs.write permissions.', |
| 144 | + })); |
| 145 | + assert.throws(() => { |
| 146 | + fs.linkSync(path.join(readOnlyFolder, 'file'), path.join(readWriteFolder, 'link-to-read-only')); |
| 147 | +@@ -37,18 +36,6 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER; |
| 148 | + resource: path.toNamespacedPath(path.join(readOnlyFolder, 'file')), |
| 149 | + })); |
| 150 | + |
| 151 | +- // App will be able to symlink to a writeOnlyFolder |
| 152 | +- fs.symlink(path.join(readWriteFolder, 'file'), path.join(writeOnlyFolder, 'link-to-read-write'), 'file', (err) => { |
| 153 | +- assert.ifError(err); |
| 154 | +- // App will won't be able to read the symlink |
| 155 | +- fs.readFile(path.join(writeOnlyFolder, 'link-to-read-write'), common.expectsError({ |
| 156 | +- code: 'ERR_ACCESS_DENIED', |
| 157 | +- permission: 'FileSystemRead', |
| 158 | +- })); |
| 159 | +- |
| 160 | +- // App will be able to write to the symlink |
| 161 | +- fs.writeFile(path.join(writeOnlyFolder, 'link-to-read-write'), 'some content', common.mustSucceed()); |
| 162 | +- }); |
| 163 | + fs.link(path.join(readWriteFolder, 'file'), path.join(writeOnlyFolder, 'link-to-read-write2'), (err) => { |
| 164 | + assert.ifError(err); |
| 165 | + // App will won't be able to read the link |
| 166 | +@@ -66,8 +53,7 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER; |
| 167 | + fs.symlinkSync(path.join(readWriteFolder, 'file'), path.join(readOnlyFolder, 'link-to-read-only'), 'file'); |
| 168 | + }, common.expectsError({ |
| 169 | + code: 'ERR_ACCESS_DENIED', |
| 170 | +- permission: 'FileSystemWrite', |
| 171 | +- resource: path.toNamespacedPath(path.join(readOnlyFolder, 'link-to-read-only')), |
| 172 | ++ message: 'fs.symlink API requires full fs.read and fs.write permissions.', |
| 173 | + })); |
| 174 | + assert.throws(() => { |
| 175 | + fs.linkSync(path.join(readWriteFolder, 'file'), path.join(readOnlyFolder, 'link-to-read-only')); |
| 176 | +diff --git a/test/fixtures/permission/fs-symlink.js b/test/fixtures/permission/fs-symlink.js |
| 177 | +index 4cf3b45f..ba60f781 100644 |
| 178 | +--- a/test/fixtures/permission/fs-symlink.js |
| 179 | ++++ b/test/fixtures/permission/fs-symlink.js |
| 180 | +@@ -54,7 +54,6 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK; |
| 181 | + fs.readFileSync(blockedFile); |
| 182 | + }, common.expectsError({ |
| 183 | + code: 'ERR_ACCESS_DENIED', |
| 184 | +- permission: 'FileSystemRead', |
| 185 | + })); |
| 186 | + assert.throws(() => { |
| 187 | + fs.appendFileSync(blockedFile, 'data'); |
| 188 | +@@ -68,7 +67,6 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK; |
| 189 | + fs.symlinkSync(regularFile, blockedFolder + '/asdf', 'file'); |
| 190 | + }, common.expectsError({ |
| 191 | + code: 'ERR_ACCESS_DENIED', |
| 192 | +- permission: 'FileSystemWrite', |
| 193 | + })); |
| 194 | + assert.throws(() => { |
| 195 | + fs.linkSync(regularFile, blockedFolder + '/asdf'); |
| 196 | +@@ -82,7 +80,6 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK; |
| 197 | + fs.symlinkSync(blockedFile, path.join(__dirname, '/asdf'), 'file'); |
| 198 | + }, common.expectsError({ |
| 199 | + code: 'ERR_ACCESS_DENIED', |
| 200 | +- permission: 'FileSystemRead', |
| 201 | + })); |
| 202 | + assert.throws(() => { |
| 203 | + fs.linkSync(blockedFile, path.join(__dirname, '/asdf')); |
| 204 | +@@ -90,4 +87,19 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK; |
| 205 | + code: 'ERR_ACCESS_DENIED', |
| 206 | + permission: 'FileSystemRead', |
| 207 | + })); |
| 208 | ++} |
| 209 | ++ |
| 210 | ++// fs.symlink API is blocked by default |
| 211 | ++{ |
| 212 | ++ assert.throws(() => { |
| 213 | ++ fs.symlinkSync(regularFile, regularFile); |
| 214 | ++ }, common.expectsError({ |
| 215 | ++ message: 'fs.symlink API requires full fs.read and fs.write permissions.', |
| 216 | ++ code: 'ERR_ACCESS_DENIED', |
| 217 | ++ })); |
| 218 | ++ |
| 219 | ++ fs.symlink(regularFile, regularFile, common.expectsError({ |
| 220 | ++ message: 'fs.symlink API requires full fs.read and fs.write permissions.', |
| 221 | ++ code: 'ERR_ACCESS_DENIED', |
| 222 | ++ })); |
| 223 | + } |
| 224 | +\ No newline at end of file |
| 225 | +diff --git a/test/parallel/test-permission-fs-symlink-relative.js b/test/parallel/test-permission-fs-symlink-relative.js |
| 226 | +index 4cc7d920..9080f16c 100644 |
| 227 | +--- a/test/parallel/test-permission-fs-symlink-relative.js |
| 228 | ++++ b/test/parallel/test-permission-fs-symlink-relative.js |
| 229 | +@@ -1,4 +1,4 @@ |
| 230 | +-// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* |
| 231 | ++// Flags: --experimental-permission --allow-fs-read=* |
| 232 | + 'use strict'; |
| 233 | + |
| 234 | + const common = require('../common'); |
| 235 | +@@ -10,7 +10,7 @@ const { symlinkSync, symlink, promises: { symlink: symlinkAsync } } = require('f |
| 236 | + |
| 237 | + const error = { |
| 238 | + code: 'ERR_ACCESS_DENIED', |
| 239 | +- message: /relative symbolic link target/, |
| 240 | ++ message: /symlink API requires full fs\.read and fs\.write permissions/, |
| 241 | + }; |
| 242 | + |
| 243 | + for (const targetString of ['a', './b/c', '../d', 'e/../f', 'C:drive-relative', 'ntfs:alternate']) { |
| 244 | +@@ -27,14 +27,14 @@ for (const targetString of ['a', './b/c', '../d', 'e/../f', 'C:drive-relative', |
| 245 | + } |
| 246 | + } |
| 247 | + |
| 248 | +-// Absolute should not throw |
| 249 | ++// Absolute should throw too |
| 250 | + for (const targetString of [path.resolve('.')]) { |
| 251 | + for (const target of [targetString, Buffer.from(targetString)]) { |
| 252 | + for (const path of [__filename]) { |
| 253 | + symlink(target, path, common.mustCall((err) => { |
| 254 | + assert(err); |
| 255 | +- assert.strictEqual(err.code, 'EEXIST'); |
| 256 | +- assert.match(err.message, /file already exists/); |
| 257 | ++ assert.strictEqual(err.code, error.code); |
| 258 | ++ assert.match(err.message, error.message); |
| 259 | + })); |
| 260 | + } |
| 261 | + } |
| 262 | +diff --git a/test/parallel/test-permission-fs-symlink.js b/test/parallel/test-permission-fs-symlink.js |
| 263 | +index c7d753c2..268a8ecb 100644 |
| 264 | +--- a/test/parallel/test-permission-fs-symlink.js |
| 265 | ++++ b/test/parallel/test-permission-fs-symlink.js |
| 266 | +@@ -21,15 +21,26 @@ const commonPathWildcard = path.join(__filename, '../../common*'); |
| 267 | + const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md'); |
| 268 | + const blockedFolder = tmpdir.resolve('subdirectory'); |
| 269 | + const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md'); |
| 270 | ++const allowedFolder = tmpdir.resolve('allowed-folder'); |
| 271 | ++const traversalSymlink = path.join(allowedFolder, 'deep1', 'deep2', 'deep3', 'gotcha'); |
| 272 | + |
| 273 | + { |
| 274 | + tmpdir.refresh(); |
| 275 | + fs.mkdirSync(blockedFolder); |
| 276 | ++ // Create deep directory structure for path traversal test |
| 277 | ++ fs.mkdirSync(allowedFolder); |
| 278 | ++ fs.writeFileSync(path.resolve(allowedFolder, '../protected-file.md'), 'protected'); |
| 279 | ++ fs.mkdirSync(path.join(allowedFolder, 'deep1')); |
| 280 | ++ fs.mkdirSync(path.join(allowedFolder, 'deep1', 'deep2')); |
| 281 | ++ fs.mkdirSync(path.join(allowedFolder, 'deep1', 'deep2', 'deep3')); |
| 282 | + } |
| 283 | + |
| 284 | + { |
| 285 | + // Symlink previously created |
| 286 | ++ // fs.symlink API is allowed when full-read and full-write access |
| 287 | + fs.symlinkSync(blockedFile, symlinkFromBlockedFile); |
| 288 | ++ // Create symlink for path traversal test - symlink points to parent directory |
| 289 | ++ fs.symlinkSync(allowedFolder, traversalSymlink); |
| 290 | + } |
| 291 | + |
| 292 | + { |
| 293 | +@@ -38,6 +49,7 @@ const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md'); |
| 294 | + [ |
| 295 | + '--experimental-permission', |
| 296 | + `--allow-fs-read=${file}`, `--allow-fs-read=${commonPathWildcard}`, `--allow-fs-read=${symlinkFromBlockedFile}`, |
| 297 | ++ `--allow-fs-read=${allowedFolder}`, |
| 298 | + `--allow-fs-write=${symlinkFromBlockedFile}`, |
| 299 | + file, |
| 300 | + ], |
| 301 | +@@ -47,6 +59,8 @@ const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md'); |
| 302 | + BLOCKEDFOLDER: blockedFolder, |
| 303 | + BLOCKEDFILE: blockedFile, |
| 304 | + EXISTINGSYMLINK: symlinkFromBlockedFile, |
| 305 | ++ TRAVERSALSYMLINK: traversalSymlink, |
| 306 | ++ ALLOWEDFOLDER: allowedFolder, |
| 307 | + }, |
| 308 | + } |
| 309 | + ); |
| 310 | +-- |
| 311 | +2.45.4 |
| 312 | + |
0 commit comments