Skip to content

Commit 9960109

Browse files
jazellyaduh95
authored andcommitted
fs: resolve symlinks before parent components
The JavaScript implementation of `fs.realpath()` and `fs.realpathSync()` joined a symlink target with the rest of the path through `path.resolve()`, which collapses `..` lexically before the preceding components have been resolved. A link such as `d -> c/../d`, where `c` is itself a symlink, was therefore rewritten back to `d` and the walk never terminated. Keep `.` and `..` in the path while walking it, and apply `..` to the already resolved parent, matching `realpath(3)` and `fs.realpathSync.native()`. Only normalize the final result, and skip caching link targets containing `..`, whose resolution depends on the path walked before them. Thsi is a POSIX only fix. Windows is unaffected. Signed-off-by: jazelly <[email protected]> PR-URL: #65457 Fixes: #60295 Reviewed-By: Jake Yuesong Li <[email protected]>
1 parent 942fd1b commit 9960109

2 files changed

Lines changed: 175 additions & 11 deletions

File tree

‎lib/fs.js‎

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3340,6 +3340,25 @@ if (isWindows) {
33403340
};
33413341
}
33423342

3343+
// realpath(3) applies `..` to the resolved parent, so on POSIX `.` and `..`
3344+
// are kept in the path while walking it. Windows applies `..` lexically, both
3345+
// in the path being opened and in symlink targets, so the path and the link
3346+
// targets are normalized up front there, matching `fs.realpathSync.native()`.
3347+
function toRealpathInput(p) {
3348+
if (isWindows) return pathModule.resolve(p);
3349+
return pathModule.isAbsolute(p) ? p : `${pathModule.resolve()}/${p}`;
3350+
}
3351+
3352+
function joinRealpathLink(previous, linkTarget) {
3353+
if (isWindows) return pathModule.resolve(previous, linkTarget);
3354+
return pathModule.isAbsolute(linkTarget) ? linkTarget : `${previous}${linkTarget}`;
3355+
}
3356+
3357+
function joinRealpathRemainder(resolvedLink, remainder) {
3358+
if (isWindows) return pathModule.resolve(resolvedLink, remainder);
3359+
return remainder.length === 0 ? resolvedLink : `${resolvedLink}/${remainder}`;
3360+
}
3361+
33433362
/**
33443363
* Returns the resolved pathname.
33453364
* @param {string | Buffer | URL} p
@@ -3358,7 +3377,7 @@ function realpathSync(p, options) {
33583377
p += '';
33593378
}
33603379
validatePath(p);
3361-
p = pathModule.resolve(p);
3380+
p = toRealpathInput(p);
33623381

33633382
// Check the resolved input path up front so that the lstat walk below cannot
33643383
// be used to probe the existence of paths outside the granted set.
@@ -3372,14 +3391,14 @@ function realpathSync(p, options) {
33723391
}
33733392

33743393
const cache = options[realpathCacheKey];
3375-
const maybeCachedResult = cache?.get(p);
3394+
const original = p;
3395+
const maybeCachedResult = cache?.get(original);
33763396
if (maybeCachedResult) {
33773397
return maybeCachedResult;
33783398
}
33793399

33803400
const seenLinks = new SafeMap();
33813401
const knownHard = new SafeSet();
3382-
const original = p;
33833402
// Whether the symlink this walk resolved last pointed at a pipe or a
33843403
// socket, which is where the walk stops. It cannot be read back from the
33853404
// shared stat buffer, which holds the last stat made anywhere in the
@@ -3415,6 +3434,9 @@ function realpathSync(p, options) {
34153434
while (pos < p.length) {
34163435
// find the next part
34173436
const result = nextPart(p, pos);
3437+
const part = result === -1 ?
3438+
StringPrototypeSlice(p, pos) :
3439+
StringPrototypeSlice(p, pos, result);
34183440
previous = current;
34193441
if (result === -1) {
34203442
const last = StringPrototypeSlice(p, pos);
@@ -3427,6 +3449,19 @@ function realpathSync(p, options) {
34273449
pos = result + 1;
34283450
}
34293451

3452+
if (part === '' || part === '.') {
3453+
current = previous;
3454+
continue;
3455+
}
3456+
if (part === '..') {
3457+
base = pathModule.dirname(previous);
3458+
current = base;
3459+
if (current[current.length - 1] !== CHAR_FORWARD_SLASH &&
3460+
current[current.length - 1] !== CHAR_BACKWARD_SLASH) {
3461+
current += pathModule.sep;
3462+
}
3463+
continue;
3464+
}
34303465
// Continue if not a symlink, break if a pipe/socket
34313466
if (knownHard.has(base) || cache?.get(base) === base) {
34323467
if (reachedPipeOrSocket) {
@@ -3472,14 +3507,18 @@ function realpathSync(p, options) {
34723507
isFileType(targetStats, S_IFSOCK);
34733508
linkTarget = binding.readlink(base, undefined);
34743509
}
3475-
resolvedLink = pathModule.resolve(previous, linkTarget);
3510+
resolvedLink = joinRealpathLink(previous, linkTarget);
34763511

3477-
cache?.set(base, resolvedLink);
3512+
// On POSIX, a target containing `..` resolves relative to the path
3513+
// walked before it, so it cannot be cached by the link alone.
3514+
if (isWindows || StringPrototypeIndexOf(linkTarget, '..') === -1) {
3515+
cache?.set(base, pathModule.resolve(resolvedLink));
3516+
}
34783517
if (!isWindows) seenLinks.set(id, linkTarget);
34793518
}
34803519

34813520
// Resolve the link, then start over
3482-
p = pathModule.resolve(resolvedLink, StringPrototypeSlice(p, pos));
3521+
p = joinRealpathRemainder(resolvedLink, StringPrototypeSlice(p, pos));
34833522

34843523
// Skip over roots
34853524
current = base = splitRoot(p);
@@ -3496,6 +3535,7 @@ function realpathSync(p, options) {
34963535
}
34973536
}
34983537

3538+
p = pathModule.resolve(p);
34993539
cache?.set(original, p);
35003540
return encodeRealpathResult(p, options);
35013541
}
@@ -3548,7 +3588,7 @@ function realpath(p, options, callback) {
35483588
p += '';
35493589
}
35503590
validatePath(p);
3551-
p = pathModule.resolve(p);
3591+
p = toRealpathInput(p);
35523592

35533593
const seenLinks = new SafeMap();
35543594
const knownHard = new SafeSet();
@@ -3586,11 +3626,14 @@ function realpath(p, options, callback) {
35863626
function LOOP() {
35873627
// Stop if scanned past end of path
35883628
if (pos >= p.length) {
3589-
return callback(null, encodeRealpathResult(p, options));
3629+
return callback(null, encodeRealpathResult(pathModule.resolve(p), options));
35903630
}
35913631

35923632
// find the next part
35933633
const result = nextPart(p, pos);
3634+
const part = result === -1 ?
3635+
StringPrototypeSlice(p, pos) :
3636+
StringPrototypeSlice(p, pos, result);
35943637
previous = current;
35953638
if (result === -1) {
35963639
const last = StringPrototypeSlice(p, pos);
@@ -3603,10 +3646,23 @@ function realpath(p, options, callback) {
36033646
pos = result + 1;
36043647
}
36053648

3649+
if (part === '' || part === '.') {
3650+
current = previous;
3651+
return process.nextTick(LOOP);
3652+
}
3653+
if (part === '..') {
3654+
base = pathModule.dirname(previous);
3655+
current = base;
3656+
if (current[current.length - 1] !== CHAR_FORWARD_SLASH &&
3657+
current[current.length - 1] !== CHAR_BACKWARD_SLASH) {
3658+
current += pathModule.sep;
3659+
}
3660+
return process.nextTick(LOOP);
3661+
}
36063662
// Continue if not a symlink, break if a pipe/socket
36073663
if (knownHard.has(base)) {
36083664
if (reachedPipeOrSocket) {
3609-
return callback(null, encodeRealpathResult(p, options));
3665+
return callback(null, encodeRealpathResult(pathModule.resolve(p), options));
36103666
}
36113667
return process.nextTick(LOOP);
36123668
}
@@ -3650,12 +3706,12 @@ function realpath(p, options, callback) {
36503706
function gotTarget(err, target) {
36513707
if (err) return callback(err);
36523708

3653-
gotResolvedLink(pathModule.resolve(previous, target));
3709+
gotResolvedLink(joinRealpathLink(previous, target));
36543710
}
36553711

36563712
function gotResolvedLink(resolvedLink) {
36573713
// Resolve the link, then start over
3658-
p = pathModule.resolve(resolvedLink, StringPrototypeSlice(p, pos));
3714+
p = joinRealpathRemainder(resolvedLink, StringPrototypeSlice(p, pos));
36593715
current = base = splitRoot(p);
36603716
pos = current.length;
36613717

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Flags: --expose-internals
2+
3+
'use strict';
4+
5+
const common = require('../common');
6+
const assert = require('assert');
7+
const { spawnSync } = require('child_process');
8+
const fs = require('fs');
9+
const { realpathCacheKey } = require('internal/fs/utils');
10+
const path = require('path');
11+
const tmpdir = require('../common/tmpdir');
12+
13+
if (process.argv[2] === 'child') {
14+
if (process.argv[4] === 'callback') {
15+
fs.realpath(process.argv[3], common.mustSucceed((result) => {
16+
process.stdout.write(result);
17+
}));
18+
} else {
19+
process.stdout.write(fs.realpathSync(process.argv[3]));
20+
}
21+
} else {
22+
// Windows applies `..` in symlink targets lexically, so `d -> c\..\d` points
23+
// at itself there and the walk fails with ELOOP, like the operating system.
24+
if (common.isWindows)
25+
common.skip('symlink targets are normalized lexically on Windows');
26+
if (!common.canCreateSymLink())
27+
common.skip('insufficient privileges');
28+
29+
tmpdir.refresh();
30+
31+
// Reproduce a symlink target containing `..` after another symlink:
32+
// c -> a/b/c
33+
// d -> c/../d -> a/b/d
34+
// The cache fixture also creates a separate d to ensure that c/../d stays
35+
// distinct from the lexically normalized path d.
36+
const target = tmpdir.resolve('a', 'b', 'd');
37+
fs.mkdirSync(tmpdir.resolve('a', 'b', 'c'), { recursive: true });
38+
fs.mkdirSync(target);
39+
fs.symlinkSync(path.join('a', 'b', 'c'), tmpdir.resolve('c'), 'dir');
40+
fs.symlinkSync(`c${path.sep}..${path.sep}d`, tmpdir.resolve('d'), 'dir');
41+
42+
const cacheRoot = tmpdir.resolve('cache');
43+
const cacheTarget = path.join(cacheRoot, 'a', 'b', 'd');
44+
const cachePlain = path.join(cacheRoot, 'd');
45+
fs.mkdirSync(path.join(cacheRoot, 'a', 'b', 'c'), { recursive: true });
46+
fs.mkdirSync(cacheTarget);
47+
fs.mkdirSync(cachePlain);
48+
fs.symlinkSync(path.join('a', 'b', 'c'), path.join(cacheRoot, 'c'), 'dir');
49+
const cacheDotted = `${cacheRoot}${path.sep}c${path.sep}..${path.sep}d`;
50+
const cacheExpected = fs.realpathSync(cacheTarget);
51+
const plainExpected = fs.realpathSync(cachePlain);
52+
53+
function assertCacheIsolation(first, firstExpected, second, secondExpected) {
54+
const cache = new Map();
55+
const options = { [realpathCacheKey]: cache };
56+
assert.strictEqual(fs.realpathSync(first, options), firstExpected);
57+
assert.strictEqual(fs.realpathSync(second, options), secondExpected);
58+
}
59+
60+
assertCacheIsolation(cacheDotted, cacheExpected, cachePlain, plainExpected);
61+
assertCacheIsolation(cachePlain, plainExpected, cacheDotted, cacheExpected);
62+
63+
// Run realpathSync in a child because the regression does not return.
64+
const result = spawnSync(process.execPath, [
65+
'--expose-internals',
66+
__filename,
67+
'child',
68+
tmpdir.resolve('d'),
69+
], {
70+
encoding: 'utf8',
71+
timeout: common.platformTimeout(5000),
72+
});
73+
74+
assert.ifError(result.error);
75+
assert.strictEqual(result.status, 0, result.stderr);
76+
assert.strictEqual(result.stdout, target);
77+
78+
const directInput = `c${path.sep}..${path.sep}d`;
79+
const directResult = spawnSync(process.execPath, [
80+
'--expose-internals',
81+
__filename,
82+
'child',
83+
directInput,
84+
], {
85+
encoding: 'utf8',
86+
cwd: tmpdir.path,
87+
timeout: common.platformTimeout(5000),
88+
});
89+
90+
assert.ifError(directResult.error);
91+
assert.strictEqual(directResult.status, 0, directResult.stderr);
92+
assert.strictEqual(directResult.stdout, target);
93+
94+
const callbackResult = spawnSync(process.execPath, [
95+
'--expose-internals',
96+
__filename,
97+
'child',
98+
tmpdir.resolve('d'),
99+
'callback',
100+
], {
101+
encoding: 'utf8',
102+
timeout: common.platformTimeout(5000),
103+
});
104+
105+
assert.ifError(callbackResult.error);
106+
assert.strictEqual(callbackResult.status, 0, callbackResult.stderr);
107+
assert.strictEqual(callbackResult.stdout, target);
108+
}

0 commit comments

Comments
 (0)