Fix copy/move recursing into source through a symlinked dest ancestor (#1073)

checkParentPaths walks up the destination's ancestors to detect when the
destination is inside the source (which would cause copy to recurse into
the tree it is creating). When an intermediate parent did not exist yet
it returned early on ENOENT, so a deeper ancestor that is a symlink into
the source tree was never checked. copy then created the missing path
under the symlink target and walked into the subtree until ENAMETOOLONG.

Recurse to the next parent on ENOENT instead of returning, so the
self-subdirectory check keeps walking up to an existing ancestor.

Author: sarathfrancis90

lib/copy/__tests__/copy-prevent-copying-into-itself.test.js

@@ -210,6 +210,29 @@ describe('+ copy() - prevent copying into itself', () => {
         })
       })
 
+      it('should error when dest is a subdirectory of src and the dest parent does not exist yet', done => {
+        const destLink = path.join(TEST_DIR, 'dest-symlink')
+        fs.symlinkSync(src, destLink, 'dir')
+
+        const srclenBefore = klawSync(src).length
+        assert(srclenBefore > 2)
+
+        // The immediate dest parent ('sub') does not exist yet, but a deeper
+        // ancestor ('dest-symlink') is a symlink into src.
+        const dest = path.join(destLink, 'sub', 'dest')
+        assert(!fs.existsSync(path.dirname(dest)))
+        fs.copy(src, dest, err => {
+          assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${path.join(destLink, 'sub')}'.`)
+
+          const srclenAfter = klawSync(src).length
+          assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change')
+
+          const link = fs.readlinkSync(destLink)
+          assert.strictEqual(link, src)
+          done()
+        })
+      })
+
       it('should copy the directory successfully when src is a subdir of resolved dest path and dereference is true', done => {
         const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src')
         const destLink = path.join(TEST_DIR, 'dest-symlink')

lib/copy/__tests__/copy-sync-prevent-copying-into-itself.test.js

@@ -216,6 +216,33 @@ describe('+ copySync() - prevent copying into itself', () => {
         }
       })
 
+      it('should error when dest is a subdirectory of src and the dest parent does not exist yet', () => {
+        const destLink = path.join(TEST_DIR, 'dest-symlink')
+        fs.symlinkSync(src, destLink, 'dir')
+
+        const srclenBefore = klawSync(src).length
+        assert(srclenBefore > 2)
+
+        // The immediate dest parent ('sub') does not exist yet, but a deeper
+        // ancestor ('dest-symlink') is a symlink into src.
+        const dest = path.join(destLink, 'sub', 'dest')
+        assert(!fs.existsSync(path.dirname(dest)))
+        let errThrown = false
+        try {
+          fs.copySync(src, dest)
+        } catch (err) {
+          assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${path.join(destLink, 'sub')}'.`)
+          errThrown = true
+        } finally {
+          assert(errThrown)
+          const srclenAfter = klawSync(src).length
+          assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change')
+
+          const link = fs.readlinkSync(destLink)
+          assert.strictEqual(link, src)
+        }
+      })
+
       it('should copy the directory successfully when src is a subdir of resolved dest path and dereferene is true', () => {
         const srcInDest = path.join(TEST_DIR, 'dest', 'some', 'nested', 'src')
         const destLink = path.join(TEST_DIR, 'dest-symlink')

lib/util/stat.js

@@ -101,7 +101,10 @@ async function checkParentPaths (src, srcStat, dest, funcName) {
   try {
     destStat = await fs.stat(destParent, { bigint: true })
   } catch (err) {
-    if (err.code === 'ENOENT') return
+    // The destination parent does not exist yet, but a deeper ancestor might
+    // (e.g. when it is a symlink into the source tree). Keep walking up so the
+    // self-subdirectory check is not bypassed.
+    if (err.code === 'ENOENT') return checkParentPaths(src, srcStat, destParent, funcName)
     throw err
   }
 
@@ -120,7 +123,10 @@ function checkParentPathsSync (src, srcStat, dest, funcName) {
   try {
     destStat = fs.statSync(destParent, { bigint: true })
   } catch (err) {
-    if (err.code === 'ENOENT') return
+    // The destination parent does not exist yet, but a deeper ancestor might
+    // (e.g. when it is a symlink into the source tree). Keep walking up so the
+    // self-subdirectory check is not bypassed.
+    if (err.code === 'ENOENT') return checkParentPathsSync(src, srcStat, destParent, funcName)
     throw err
   }
   if (areIdentical(srcStat, destStat)) {