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 26be208

Browse filesBrowse files
RafaelGSSRafaelGSS
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 f0a8916 commit 26be208
Copy full SHA for 26be208

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');
@@ -1777,18 +1776,12 @@ function symlink(target, path, type, callback) {
17771776
validateOneOf(type, 'type', ['dir', 'file', 'junction', null, undefined]);
17781777
}
17791778

1780-
if (permission.isEnabled()) {
1781-
// The permission model's security guarantees fall apart in the presence of
1782-
// relative symbolic links. Thus, we have to prevent their creation.
1783-
if (BufferIsBuffer(target)) {
1784-
if (!isAbsolute(BufferToString(target))) {
1785-
callback(new ERR_ACCESS_DENIED('relative symbolic link target'));
1786-
return;
1787-
}
1788-
} else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
1789-
callback(new ERR_ACCESS_DENIED('relative symbolic link target'));
1790-
return;
1791-
}
1779+
// Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass
1780+
// the permission model security guarantees. Thus, this API is disabled unless fs.read
1781+
// and fs.write permission has been given.
1782+
if (permission.isEnabled() && !permission.has('fs')) {
1783+
callback(new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.'));
1784+
return;
17921785
}
17931786

17941787
target = getValidatedPath(target, 'target');
@@ -1852,16 +1845,11 @@ function symlinkSync(target, path, type) {
18521845
}
18531846
}
18541847

1855-
if (permission.isEnabled()) {
1856-
// The permission model's security guarantees fall apart in the presence of
1857-
// relative symbolic links. Thus, we have to prevent their creation.
1858-
if (BufferIsBuffer(target)) {
1859-
if (!isAbsolute(BufferToString(target))) {
1860-
throw new ERR_ACCESS_DENIED('relative symbolic link target');
1861-
}
1862-
} else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
1863-
throw new ERR_ACCESS_DENIED('relative symbolic link target');
1864-
}
1848+
// Due to the nature of Node.js runtime, symlinks has different edge cases that can bypass
1849+
// the permission model security guarantees. Thus, this API is disabled unless fs.read
1850+
// and fs.write permission has been given.
1851+
if (permission.isEnabled() && !permission.has('fs')) {
1852+
throw new ERR_ACCESS_DENIED('fs.symlink API requires full fs.read and fs.write permissions.');
18651853
}
18661854

18671855
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
@@ -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,
@@ -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
getLazy,
9590
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');
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.