nodejs/node · #65457

fs: resolve symlinks before parent components

jazelly · merged Nov 10, 20262 files · 175 + / 11 −
lib/fs.js67 + / 11 −
@@ -3339,6 +3339,25 @@ if (isWindows) {   }; } +// realpath(3) applies `..` to the resolved parent, so on POSIX `.` and `..`+// are kept in the path while walking it. Windows applies `..` lexically, both+// in the path being opened and in symlink targets, so the path and the link+// targets are normalized up front there, matching `fs.realpathSync.native()`.+function toRealpathInput(p) {+  if (isWindows) return pathModule.resolve(p);+  return pathModule.isAbsolute(p) ? p : `${pathModule.resolve()}/${p}`;+}++function joinRealpathLink(previous, linkTarget) {+  if (isWindows) return pathModule.resolve(previous, linkTarget);+  return pathModule.isAbsolute(linkTarget) ? linkTarget : `${previous}${linkTarget}`;+}++function joinRealpathRemainder(resolvedLink, remainder) {+  if (isWindows) return pathModule.resolve(resolvedLink, remainder);+  return remainder.length === 0 ? resolvedLink : `${resolvedLink}/${remainder}`;+}+ /**  * Returns the resolved pathname.  * @param {string | Buffer | URL} p@@ -3357,7 +3376,7 @@ function realpathSync(p, options) {     p += '';   }   validatePath(p);-  p = pathModule.resolve(p);+  p = toRealpathInput(p);    // Check the resolved input path up front so that the lstat walk below cannot   // be used to probe the existence of paths outside the granted set.@@ -3371,14 +3390,14 @@ function realpathSync(p, options) {   }    const cache = options[realpathCacheKey];-  const maybeCachedResult = cache?.get(p);+  const original = p;+  const maybeCachedResult = cache?.get(original);   if (maybeCachedResult) {     return maybeCachedResult;   }    const seenLinks = new SafeMap();   const knownHard = new SafeSet();-  const original = p;   // Whether the symlink this walk resolved last pointed at a pipe or a   // socket, which is where the walk stops. It cannot be read back from the   // shared stat buffer, which holds the last stat made anywhere in the@@ -3414,6 +3433,9 @@ function realpathSync(p, options) {   while (pos < p.length) {     // find the next part     const result = nextPart(p, pos);+    const part = result === -1 ?+      StringPrototypeSlice(p, pos) :+      StringPrototypeSlice(p, pos, result);     previous = current;     if (result === -1) {       const last = StringPrototypeSlice(p, pos);@@ -3426,6 +3448,19 @@ function realpathSync(p, options) {       pos = result + 1;     } +    if (part === '' || part === '.') {+      current = previous;+      continue;+    }+    if (part === '..') {+      base = pathModule.dirname(previous);+      current = base;+      if (current[current.length - 1] !== CHAR_FORWARD_SLASH &&+          current[current.length - 1] !== CHAR_BACKWARD_SLASH) {+        current += pathModule.sep;+      }+      continue;+    }     // Continue if not a symlink, break if a pipe/socket     if (knownHard.has(base) || cache?.get(base) === base) {       if (reachedPipeOrSocket) {@@ -3471,14 +3506,18 @@ function realpathSync(p, options) {           isFileType(targetStats, S_IFSOCK);         linkTarget = binding.readlink(base, undefined);       }-      resolvedLink = pathModule.resolve(previous, linkTarget);+      resolvedLink = joinRealpathLink(previous, linkTarget); -      cache?.set(base, resolvedLink);+      // On POSIX, a target containing `..` resolves relative to the path+      // walked before it, so it cannot be cached by the link alone.+      if (isWindows || StringPrototypeIndexOf(linkTarget, '..') === -1) {+        cache?.set(base, pathModule.resolve(resolvedLink));+      }       if (!isWindows) seenLinks.set(id, linkTarget);     }      // Resolve the link, then start over-    p = pathModule.resolve(resolvedLink, StringPrototypeSlice(p, pos));+    p = joinRealpathRemainder(resolvedLink, StringPrototypeSlice(p, pos));      // Skip over roots     current = base = splitRoot(p);@@ -3495,6 +3534,7 @@ function realpathSync(p, options) {     }   } +  p = pathModule.resolve(p);   cache?.set(original, p);   return encodeRealpathResult(p, options); }@@ -3547,7 +3587,7 @@ function realpath(p, options, callback) {     p += '';   }   validatePath(p);-  p = pathModule.resolve(p);+  p = toRealpathInput(p);    const seenLinks = new SafeMap();   const knownHard = new SafeSet();@@ -3585,11 +3625,14 @@ function realpath(p, options, callback) {   function LOOP() {     // Stop if scanned past end of path     if (pos >= p.length) {-      return callback(null, encodeRealpathResult(p, options));+      return callback(null, encodeRealpathResult(pathModule.resolve(p), options));     }      // find the next part     const result = nextPart(p, pos);+    const part = result === -1 ?+      StringPrototypeSlice(p, pos) :+      StringPrototypeSlice(p, pos, result);     previous = current;     if (result === -1) {       const last = StringPrototypeSlice(p, pos);@@ -3602,10 +3645,23 @@ function realpath(p, options, callback) {       pos = result + 1;     } +    if (part === '' || part === '.') {+      current = previous;+      return process.nextTick(LOOP);+    }+    if (part === '..') {+      base = pathModule.dirname(previous);+      current = base;+      if (current[current.length - 1] !== CHAR_FORWARD_SLASH &&+          current[current.length - 1] !== CHAR_BACKWARD_SLASH) {+        current += pathModule.sep;+      }+      return process.nextTick(LOOP);+    }     // Continue if not a symlink, break if a pipe/socket     if (knownHard.has(base)) {       if (reachedPipeOrSocket) {-        return callback(null, encodeRealpathResult(p, options));+        return callback(null, encodeRealpathResult(pathModule.resolve(p), options));       }       return process.nextTick(LOOP);     }@@ -3649,12 +3705,12 @@ function realpath(p, options, callback) {   function gotTarget(err, target) {     if (err) return callback(err); -    gotResolvedLink(pathModule.resolve(previous, target));+    gotResolvedLink(joinRealpathLink(previous, target));   }    function gotResolvedLink(resolvedLink) {     // Resolve the link, then start over-    p = pathModule.resolve(resolvedLink, StringPrototypeSlice(p, pos));+    p = joinRealpathRemainder(resolvedLink, StringPrototypeSlice(p, pos));     current = base = splitRoot(p);     pos = current.length; 
test/parallel/test-fs-realpath-symlink-dot-dot.jsadded108 + / 0 −
@@ -0,0 +1,108 @@+// Flags: --expose-internals++'use strict';++const common = require('../common');+const assert = require('assert');+const { spawnSync } = require('child_process');+const fs = require('fs');+const { realpathCacheKey } = require('internal/fs/utils');+const path = require('path');+const tmpdir = require('../common/tmpdir');++if (process.argv[2] === 'child') {+  if (process.argv[4] === 'callback') {+    fs.realpath(process.argv[3], common.mustSucceed((result) => {+      process.stdout.write(result);+    }));+  } else {+    process.stdout.write(fs.realpathSync(process.argv[3]));+  }+} else {+  // Windows applies `..` in symlink targets lexically, so `d -> c\..\d` points+  // at itself there and the walk fails with ELOOP, like the operating system.+  if (common.isWindows)+    common.skip('symlink targets are normalized lexically on Windows');+  if (!common.canCreateSymLink())+    common.skip('insufficient privileges');++  tmpdir.refresh();++  // Reproduce a symlink target containing `..` after another symlink:+  //   c -> a/b/c+  //   d -> c/../d -> a/b/d+  // The cache fixture also creates a separate d to ensure that c/../d stays+  // distinct from the lexically normalized path d.+  const target = tmpdir.resolve('a', 'b', 'd');+  fs.mkdirSync(tmpdir.resolve('a', 'b', 'c'), { recursive: true });+  fs.mkdirSync(target);+  fs.symlinkSync(path.join('a', 'b', 'c'), tmpdir.resolve('c'), 'dir');+  fs.symlinkSync(`c${path.sep}..${path.sep}d`, tmpdir.resolve('d'), 'dir');++  const cacheRoot = tmpdir.resolve('cache');+  const cacheTarget = path.join(cacheRoot, 'a', 'b', 'd');+  const cachePlain = path.join(cacheRoot, 'd');+  fs.mkdirSync(path.join(cacheRoot, 'a', 'b', 'c'), { recursive: true });+  fs.mkdirSync(cacheTarget);+  fs.mkdirSync(cachePlain);+  fs.symlinkSync(path.join('a', 'b', 'c'), path.join(cacheRoot, 'c'), 'dir');+  const cacheDotted = `${cacheRoot}${path.sep}c${path.sep}..${path.sep}d`;+  const cacheExpected = fs.realpathSync(cacheTarget);+  const plainExpected = fs.realpathSync(cachePlain);++  function assertCacheIsolation(first, firstExpected, second, secondExpected) {+    const cache = new Map();+    const options = { [realpathCacheKey]: cache };+    assert.strictEqual(fs.realpathSync(first, options), firstExpected);+    assert.strictEqual(fs.realpathSync(second, options), secondExpected);+  }++  assertCacheIsolation(cacheDotted, cacheExpected, cachePlain, plainExpected);+  assertCacheIsolation(cachePlain, plainExpected, cacheDotted, cacheExpected);++  // Run realpathSync in a child because the regression does not return.+  const result = spawnSync(process.execPath, [+    '--expose-internals',+    __filename,+    'child',+    tmpdir.resolve('d'),+  ], {+    encoding: 'utf8',+    timeout: common.platformTimeout(5000),+  });++  assert.ifError(result.error);+  assert.strictEqual(result.status, 0, result.stderr);+  assert.strictEqual(result.stdout, target);++  const directInput = `c${path.sep}..${path.sep}d`;+  const directResult = spawnSync(process.execPath, [+    '--expose-internals',+    __filename,+    'child',+    directInput,+  ], {+    encoding: 'utf8',+    cwd: tmpdir.path,+    timeout: common.platformTimeout(5000),+  });++  assert.ifError(directResult.error);+  assert.strictEqual(directResult.status, 0, directResult.stderr);+  assert.strictEqual(directResult.stdout, target);++  const callbackResult = spawnSync(process.execPath, [+    '--expose-internals',+    __filename,+    'child',+    tmpdir.resolve('d'),+    'callback',+  ], {+    encoding: 'utf8',+    timeout: common.platformTimeout(5000),+  });++  assert.ifError(callbackResult.error);+  assert.strictEqual(callbackResult.status, 0, callbackResult.stderr);+  assert.strictEqual(callbackResult.stdout, target);+}