Skip to content

Commit 7302b4d

Browse files
RafaelGSSRafaelGSS
committed
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
1 parent 89adaa2 commit 7302b4d

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
@@ -62,7 +62,6 @@ const {
6262
} = constants;
6363

6464
const pathModule = require('path');
65-
const { isAbsolute } = pathModule;
6665
const { isArrayBufferView } = require('internal/util/types');
6766

6867
const binding = internalBinding('fs');
@@ -1801,18 +1800,12 @@ function symlink(target, path, type, callback) {
18011800
validateOneOf(type, 'type', ['dir', 'file', 'junction', null, undefined]);
18021801
}
18031802

1804-
if (permission.isEnabled()) {
1805-
// The permission model's security guarantees fall apart in the presence of
1806-
// relative symbolic links. Thus, we have to prevent their creation.
1807-
if (BufferIsBuffer(target)) {
1808-
if (!isAbsolute(BufferToString(target))) {
1809-
callback(new ERR_ACCESS_DENIED('relative symbolic link target'));
1810-
return;
1811-
}
1812-
} else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
1813-
callback(new ERR_ACCESS_DENIED('relative symbolic link target'));
1814-
return;
1815-
}
1803+
// Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass
1804+
// the permission model security guarantees. Thus, this API is disabled unless fs.read
1805+
// and fs.write permission has been given.
1806+
if (permission.isEnabled() && !permission.has('fs')) {
1807+
callback(new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.'));
1808+
return;
18161809
}
18171810

18181811
target = getValidatedPath(target, 'target');
@@ -1876,16 +1869,11 @@ function symlinkSync(target, path, type) {
18761869
}
18771870
}
18781871

1879-
if (permission.isEnabled()) {
1880-
// The permission model's security guarantees fall apart in the presence of
1881-
// relative symbolic links. Thus, we have to prevent their creation.
1882-
if (BufferIsBuffer(target)) {
1883-
if (!isAbsolute(BufferToString(target))) {
1884-
throw new ERR_ACCESS_DENIED('relative symbolic link target');
1885-
}
1886-
} else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
1887-
throw new ERR_ACCESS_DENIED('relative symbolic link target');
1888-
}
1872+
// Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass
1873+
// the permission model security guarantees. Thus, this API is disabled unless fs.read
1874+
// and fs.write permission has been given.
1875+
if (permission.isEnabled() && !permission.has('fs')) {
1876+
throw new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.');
18891877
}
18901878

18911879
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
SymbolAsyncDispose,
1919
Uint8Array,
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
AbortError,
@@ -89,8 +86,6 @@ const {
8986
kValidateObjectAllowNullable,
9087
} = require('internal/validators');
9188
const pathModule = require('path');
92-
const { isAbsolute } = pathModule;
93-
const { toPathIfFileURL } = require('internal/url');
9489
const {
9590
getLazy,
9691
kEmptyObject,
@@ -992,16 +987,11 @@ async function symlink(target, path, type) {
992987
}
993988
}
994989

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

1007997
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: --permission --allow-fs-read=* --allow-fs-write=*
1+
// Flags: --permission --allow-fs-read=*
22
'use strict';
33

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

1616
const error = {
1717
code: 'ERR_ACCESS_DENIED',
18-
message: /relative symbolic link target/,
18+
message: /symlink API requires full fs\.read and fs\.write permissions/,
1919
};
2020

2121
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',
3232
}
3333
}
3434

35-
// Absolute should not throw
35+
// Absolute should throw too
3636
for (const targetString of [path.resolve('.')]) {
3737
for (const target of [targetString, Buffer.from(targetString)]) {
3838
for (const path of [__filename]) {
3939
symlink(target, path, common.mustCall((err) => {
4040
assert(err);
41-
assert.strictEqual(err.code, 'EEXIST');
42-
assert.match(err.message, /file already exists/);
41+
assert.strictEqual(err.code, error.code);
42+
assert.match(err.message, error.message);
4343
}));
4444
}
4545
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,26 @@ const commonPathWildcard = path.join(__filename, '../../common*');
2727
const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md');
2828
const blockedFolder = tmpdir.resolve('subdirectory');
2929
const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md');
30+
const allowedFolder = tmpdir.resolve('allowed-folder');
31+
const traversalSymlink = path.join(allowedFolder, 'deep1', 'deep2', 'deep3', 'gotcha');
3032

3133
{
3234
tmpdir.refresh();
3335
fs.mkdirSync(blockedFolder);
36+
// Create deep directory structure for path traversal test
37+
fs.mkdirSync(allowedFolder);
38+
fs.writeFileSync(path.resolve(allowedFolder, '../protected-file.md'), 'protected');
39+
fs.mkdirSync(path.join(allowedFolder, 'deep1'));
40+
fs.mkdirSync(path.join(allowedFolder, 'deep1', 'deep2'));
41+
fs.mkdirSync(path.join(allowedFolder, 'deep1', 'deep2', 'deep3'));
3442
}
3543

3644
{
3745
// Symlink previously created
46+
// fs.symlink API is allowed when full-read and full-write access
3847
fs.symlinkSync(blockedFile, symlinkFromBlockedFile);
48+
// Create symlink for path traversal test - symlink points to parent directory
49+
fs.symlinkSync(allowedFolder, traversalSymlink);
3950
}
4051

4152
{
@@ -44,6 +55,7 @@ const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md');
4455
[
4556
'--permission',
4657
`--allow-fs-read=${file}`, `--allow-fs-read=${commonPathWildcard}`, `--allow-fs-read=${symlinkFromBlockedFile}`,
58+
`--allow-fs-read=${allowedFolder}`,
4759
`--allow-fs-write=${symlinkFromBlockedFile}`,
4860
file,
4961
],
@@ -53,6 +65,8 @@ const symlinkFromBlockedFile = tmpdir.resolve('example-symlink.md');
5365
BLOCKEDFOLDER: blockedFolder,
5466
BLOCKEDFILE: blockedFile,
5567
EXISTINGSYMLINK: symlinkFromBlockedFile,
68+
TRAVERSALSYMLINK: traversalSymlink,
69+
ALLOWEDFOLDER: allowedFolder,
5670
},
5771
}
5872
);

0 commit comments

Comments
 (0)