Skip to content

Commit 26be208

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

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');
@@ -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');

‎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,
@@ -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');

‎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)