Skip to content

Commit 494f62d

Browse files
RafaelGSSRafaelGSS
committed
lib,permission: require full read and write to symlink APIs
Refs: https://hackerone.com/reports/3417819 PR-URL: nodejs-private/node-private#760 Reviewed-By: Matteo Collina <[email protected]> CVE-ID: CVE-2025-55130 Signed-off-by: RafaelGSS <[email protected]>
1 parent 14fbbb5 commit 494f62d

File tree

6 files changed

+52
-62
lines changed

6 files changed

+52
-62
lines changed

‎lib/fs.js‎

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ const {
5959
} = constants;
6060

6161
const pathModule = require('path');
62-
const { isAbsolute } = pathModule;
6362
const { isArrayBufferView } = require('internal/util/types');
6463

6564
const binding = internalBinding('fs');
@@ -1745,18 +1744,12 @@ function symlink(target, path, type_, callback_) {
17451744
const type = (typeof type_ === 'string' ? type_ : null);
17461745
const callback = makeCallback(arguments[arguments.length - 1]);
17471746

1748-
if (permission.isEnabled()) {
1749-
// The permission model's security guarantees fall apart in the presence of
1750-
// relative symbolic links. Thus, we have to prevent their creation.
1751-
if (BufferIsBuffer(target)) {
1752-
if (!isAbsolute(BufferToString(target))) {
1753-
callback(new ERR_ACCESS_DENIED('relative symbolic link target'));
1754-
return;
1755-
}
1756-
} else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
1757-
callback(new ERR_ACCESS_DENIED('relative symbolic link target'));
1758-
return;
1759-
}
1747+
// Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass
1748+
// the permission model security guarantees. Thus, this API is disabled unless fs.read
1749+
// and fs.write permission has been given.
1750+
if (permission.isEnabled() && !permission.has('fs')) {
1751+
callback(new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.'));
1752+
return;
17601753
}
17611754

17621755
target = getValidatedPath(target, 'target');
@@ -1816,16 +1809,11 @@ function symlinkSync(target, path, type) {
18161809
}
18171810
}
18181811

1819-
if (permission.isEnabled()) {
1820-
// The permission model's security guarantees fall apart in the presence of
1821-
// relative symbolic links. Thus, we have to prevent their creation.
1822-
if (BufferIsBuffer(target)) {
1823-
if (!isAbsolute(BufferToString(target))) {
1824-
throw new ERR_ACCESS_DENIED('relative symbolic link target');
1825-
}
1826-
} else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
1827-
throw new ERR_ACCESS_DENIED('relative symbolic link target');
1828-
}
1812+
// Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass
1813+
// the permission model security guarantees. Thus, this API is disabled unless fs.read
1814+
// and fs.write permission has been given.
1815+
if (permission.isEnabled() && !permission.has('fs')) {
1816+
throw new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.');
18291817
}
18301818

18311819
target = getValidatedPath(target, 'target');

‎lib/internal/fs/promises.js‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ const {
1717
Symbol,
1818
Uint8Array,
1919
FunctionPrototypeBind,
20-
uncurryThis,
2120
} = primordials;
2221

2322
const { fs: constants } = internalBinding('constants');
@@ -31,8 +30,6 @@ const {
3130

3231
const binding = internalBinding('fs');
3332
const { Buffer } = require('buffer');
34-
const { isBuffer: BufferIsBuffer } = Buffer;
35-
const BufferToString = uncurryThis(Buffer.prototype.toString);
3633

3734
const {
3835
codes: {
@@ -88,8 +85,6 @@ const {
8885
kValidateObjectAllowNullable,
8986
} = require('internal/validators');
9087
const pathModule = require('path');
91-
const { isAbsolute } = pathModule;
92-
const { toPathIfFileURL } = require('internal/url');
9388
const {
9489
kEmptyObject,
9590
lazyDOMException,
@@ -987,16 +982,11 @@ async function symlink(target, path, type_) {
987982
}
988983
}
989984

990-
if (permission.isEnabled()) {
991-
// The permission model's security guarantees fall apart in the presence of
992-
// relative symbolic links. Thus, we have to prevent their creation.
993-
if (BufferIsBuffer(target)) {
994-
if (!isAbsolute(BufferToString(target))) {
995-
throw new ERR_ACCESS_DENIED('relative symbolic link target');
996-
}
997-
} else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
998-
throw new ERR_ACCESS_DENIED('relative symbolic link target');
999-
}
985+
// Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass
986+
// the permission model security guarantees. Thus, this API is disabled unless fs.read
987+
// and fs.write permission has been given.
988+
if (permission.isEnabled() && !permission.has('fs')) {
989+
throw new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.');
1000990
}
1001991

1002992
target = getValidatedPath(target, 'target');

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

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER;
2626
fs.symlinkSync(path.join(readOnlyFolder, 'file'), path.join(readWriteFolder, 'link-to-read-only'), 'file');
2727
}, common.expectsError({
2828
code: 'ERR_ACCESS_DENIED',
29-
permission: 'FileSystemWrite',
30-
resource: path.toNamespacedPath(path.join(readOnlyFolder, 'file')),
29+
message: 'fs.symlink API requires full fs.read and fs.write permissions.',
3130
}));
3231
assert.throws(() => {
3332
fs.linkSync(path.join(readOnlyFolder, 'file'), path.join(readWriteFolder, 'link-to-read-only'));
@@ -37,18 +36,6 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER;
3736
resource: path.toNamespacedPath(path.join(readOnlyFolder, 'file')),
3837
}));
3938

40-
// App will be able to symlink to a writeOnlyFolder
41-
fs.symlink(path.join(readWriteFolder, 'file'), path.join(writeOnlyFolder, 'link-to-read-write'), 'file', (err) => {
42-
assert.ifError(err);
43-
// App will won't be able to read the symlink
44-
fs.readFile(path.join(writeOnlyFolder, 'link-to-read-write'), common.expectsError({
45-
code: 'ERR_ACCESS_DENIED',
46-
permission: 'FileSystemRead',
47-
}));
48-
49-
// App will be able to write to the symlink
50-
fs.writeFile(path.join(writeOnlyFolder, 'link-to-read-write'), 'some content', common.mustSucceed());
51-
});
5239
fs.link(path.join(readWriteFolder, 'file'), path.join(writeOnlyFolder, 'link-to-read-write2'), (err) => {
5340
assert.ifError(err);
5441
// App will won't be able to read the link
@@ -66,8 +53,7 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER;
6653
fs.symlinkSync(path.join(readWriteFolder, 'file'), path.join(readOnlyFolder, 'link-to-read-only'), 'file');
6754
}, common.expectsError({
6855
code: 'ERR_ACCESS_DENIED',
69-
permission: 'FileSystemWrite',
70-
resource: path.toNamespacedPath(path.join(readOnlyFolder, 'link-to-read-only')),
56+
message: 'fs.symlink API requires full fs.read and fs.write permissions.',
7157
}));
7258
assert.throws(() => {
7359
fs.linkSync(path.join(readWriteFolder, 'file'), path.join(readOnlyFolder, 'link-to-read-only'));

‎test/fixtures/permission/fs-symlink.js‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK;
5454
fs.readFileSync(blockedFile);
5555
}, common.expectsError({
5656
code: 'ERR_ACCESS_DENIED',
57-
permission: 'FileSystemRead',
5857
}));
5958
assert.throws(() => {
6059
fs.appendFileSync(blockedFile, 'data');
@@ -68,7 +67,6 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK;
6867
fs.symlinkSync(regularFile, blockedFolder + '/asdf', 'file');
6968
}, common.expectsError({
7069
code: 'ERR_ACCESS_DENIED',
71-
permission: 'FileSystemWrite',
7270
}));
7371
assert.throws(() => {
7472
fs.linkSync(regularFile, blockedFolder + '/asdf');
@@ -82,12 +80,26 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK;
8280
fs.symlinkSync(blockedFile, path.join(__dirname, '/asdf'), 'file');
8381
}, common.expectsError({
8482
code: 'ERR_ACCESS_DENIED',
85-
permission: 'FileSystemRead',
8683
}));
8784
assert.throws(() => {
8885
fs.linkSync(blockedFile, path.join(__dirname, '/asdf'));
8986
}, common.expectsError({
9087
code: 'ERR_ACCESS_DENIED',
9188
permission: 'FileSystemRead',
9289
}));
90+
}
91+
92+
// fs.symlink API is blocked by default
93+
{
94+
assert.throws(() => {
95+
fs.symlinkSync(regularFile, regularFile);
96+
}, common.expectsError({
97+
message: 'fs.symlink API requires full fs.read and fs.write permissions.',
98+
code: 'ERR_ACCESS_DENIED',
99+
}));
100+
101+
fs.symlink(regularFile, regularFile, common.expectsError({
102+
message: 'fs.symlink API requires full fs.read and fs.write permissions.',
103+
code: 'ERR_ACCESS_DENIED',
104+
}));
93105
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=*
1+
// Flags: --experimental-permission --allow-fs-read=*
22
'use strict';
33

44
const common = require('../common');
@@ -10,7 +10,7 @@ const { symlinkSync, symlink, promises: { symlink: symlinkAsync } } = require('f
1010

1111
const error = {
1212
code: 'ERR_ACCESS_DENIED',
13-
message: /relative symbolic link target/,
13+
message: /symlink API requires full fs\.read and fs\.write permissions/,
1414
};
1515

1616
for (const targetString of ['a', './b/c', '../d', 'e/../f', 'C:drive-relative', 'ntfs:alternate']) {
@@ -27,14 +27,14 @@ for (const targetString of ['a', './b/c', '../d', 'e/../f', 'C:drive-relative',
2727
}
2828
}
2929

30-
// Absolute should not throw
30+
// Absolute should throw too
3131
for (const targetString of [path.resolve('.')]) {
3232
for (const target of [targetString, Buffer.from(targetString)]) {
3333
for (const path of [__filename]) {
3434
symlink(target, path, common.mustCall((err) => {
3535
assert(err);
36-
assert.strictEqual(err.code, 'EEXIST');
37-
assert.match(err.message, /file already exists/);
36+
assert.strictEqual(err.code, error.code);
37+
assert.match(err.message, error.message);
3838
}));
3939
}
4040
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,26 @@ const commonPathWildcard = path.join(__filename, '../../common*');
2121
const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md');
2222
const blockedFolder = tmpdir.resolve('subdirectory');
2323
const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md');
24+
const allowedFolder = tmpdir.resolve('allowed-folder');
25+
const traversalSymlink = path.join(allowedFolder, 'deep1', 'deep2', 'deep3', 'gotcha');
2426

2527
{
2628
tmpdir.refresh();
2729
fs.mkdirSync(blockedFolder);
30+
// Create deep directory structure for path traversal test
31+
fs.mkdirSync(allowedFolder);
32+
fs.writeFileSync(path.resolve(allowedFolder, '../protected-file.md'), 'protected');
33+
fs.mkdirSync(path.join(allowedFolder, 'deep1'));
34+
fs.mkdirSync(path.join(allowedFolder, 'deep1', 'deep2'));
35+
fs.mkdirSync(path.join(allowedFolder, 'deep1', 'deep2', 'deep3'));
2836
}
2937

3038
{
3139
// Symlink previously created
40+
// fs.symlink API is allowed when full-read and full-write access
3241
fs.symlinkSync(blockedFile, symlinkFromBlockedFile);
42+
// Create symlink for path traversal test - symlink points to parent directory
43+
fs.symlinkSync(allowedFolder, traversalSymlink);
3344
}
3445

3546
{
@@ -38,6 +49,7 @@ const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md');
3849
[
3950
'--experimental-permission',
4051
`--allow-fs-read=${file}`, `--allow-fs-read=${commonPathWildcard}`, `--allow-fs-read=${symlinkFromBlockedFile}`,
52+
`--allow-fs-read=${allowedFolder}`,
4153
`--allow-fs-write=${symlinkFromBlockedFile}`,
4254
file,
4355
],
@@ -47,6 +59,8 @@ const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md');
4759
BLOCKEDFOLDER: blockedFolder,
4860
BLOCKEDFILE: blockedFile,
4961
EXISTINGSYMLINK: symlinkFromBlockedFile,
62+
TRAVERSALSYMLINK: traversalSymlink,
63+
ALLOWEDFOLDER: allowedFolder,
5064
},
5165
}
5266
);

0 commit comments

Comments
 (0)