Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 6b48495

Browse filesBrowse files
RafaelGSSmarco-ippolito
RafaelGSS
authored and
marco-ippolito
committed
lib,permission: require full read and write to symlink APIs
Refs: https://hackerone.com/reports/3417819 Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: nodejs-private/node-private#760 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> CVE-ID: CVE-2025-55130
1 parent ebbf942 commit 6b48495
Copy full SHA for 6b48495

File tree

Expand file treeCollapse file tree

6 files changed

+52
-62
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+52
-62
lines changed
Open diff view settings
Collapse file

‎lib/fs.js‎

Copy file name to clipboardExpand all lines: lib/fs.js
+11-23Lines 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');
@@ -1811,18 +1810,12 @@ function symlink(target, path, type_, callback_) {
18111810
const type = (typeof type_ === 'string' ? type_ : null);
18121811
const callback = makeCallback(arguments[arguments.length - 1]);
18131812

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

18281821
target = getValidatedPath(target, 'target');
@@ -1886,16 +1879,11 @@ function symlinkSync(target, path, type) {
18861879
}
18871880
}
18881881

1889-
if (permission.isEnabled()) {
1890-
// The permission model's security guarantees fall apart in the presence of
1891-
// relative symbolic links. Thus, we have to prevent their creation.
1892-
if (BufferIsBuffer(target)) {
1893-
if (!isAbsolute(BufferToString(target))) {
1894-
throw new ERR_ACCESS_DENIED('relative symbolic link target');
1895-
}
1896-
} else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
1897-
throw new ERR_ACCESS_DENIED('relative symbolic link target');
1898-
}
1882+
// Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass
1883+
// the permission model security guarantees. Thus, this API is disabled unless fs.read
1884+
// and fs.write permission has been given.
1885+
if (permission.isEnabled() && !permission.has('fs')) {
1886+
throw new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.');
18991887
}
19001888

19011889
target = getValidatedPath(target, 'target');
Collapse file

‎lib/internal/fs/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/promises.js
+5-15Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ const {
1616
SafePromisePrototypeFinally,
1717
Symbol,
1818
Uint8Array,
19-
uncurryThis,
2019
} = primordials;
2120

2221
const { fs: constants } = internalBinding('constants');
@@ -30,8 +29,6 @@ const {
3029

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

3633
const {
3734
AbortError,
@@ -87,8 +84,6 @@ const {
8784
kValidateObjectAllowNullable,
8885
} = require('internal/validators');
8986
const pathModule = require('path');
90-
const { isAbsolute } = pathModule;
91-
const { toPathIfFileURL } = require('internal/url');
9287
const {
9388
getLazy,
9489
kEmptyObject,
@@ -991,16 +986,11 @@ async function symlink(target, path, type_) {
991986
}
992987
}
993988

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

1006996
target = getValidatedPath(target, 'target');
Collapse file

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

Copy file name to clipboardExpand all lines: test/fixtures/permission/fs-symlink-target-write.js
+2-16Lines 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'));
Collapse file

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

Copy file name to clipboardExpand all lines: test/fixtures/permission/fs-symlink.js
+15-3Lines 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
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-permission-fs-symlink-relative.js
+5-5Lines 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
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-permission-fs-symlink.js
+14Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.