From 926c88cfc4c9c34b9e31fe13121f82b3ca58104f Mon Sep 17 00:00:00 2001
From: Jakob Borg <jakob@kastelo.net>
Date: Thu, 5 Jan 2017 13:26:29 +0000
Subject: [PATCH] lib/scanner: Never, ever descend into symlinks (ref #3857)

On Windows we would descend into SYMLINKD type links when we scanned
them successfully, as we would return nil from the walk function and the
filepath.Walk iterator apparently thought it OK to descend into the
symlinked directory.

With this change we always return filepath.SkipDir no matter what.

Tested on Windows 10 as admin, does what it should.

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3875
---
 lib/scanner/walk.go | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/lib/scanner/walk.go b/lib/scanner/walk.go
index 6eaa5ee1..e2505046 100644
--- a/lib/scanner/walk.go
+++ b/lib/scanner/walk.go
@@ -278,11 +278,11 @@ func (w *walker) walkAndHashFiles(fchan, dchan chan protocol.FileInfo) filepath.
 
 		switch {
 		case info.Mode()&os.ModeSymlink == os.ModeSymlink:
-			var shouldSkip bool
-			shouldSkip, err = w.walkSymlink(absPath, relPath, dchan)
-			if err == nil && shouldSkip {
-				return skip
+			if err := w.walkSymlink(absPath, relPath, dchan); err != nil {
+				return err
 			}
+			// under no circumstances shall we descend into a symlink
+			return filepath.SkipDir
 
 		case info.Mode().IsDir():
 			err = w.walkDir(relPath, info, dchan)
@@ -376,17 +376,14 @@ func (w *walker) walkDir(relPath string, info os.FileInfo, dchan chan protocol.F
 	return nil
 }
 
-// walkSymlinks returns true if the symlink should be skipped, or an error if
-// we should stop walking altogether. filepath.Walk isn't supposed to
-// transcend into symlinks at all, but there are rumours that this may have
-// happened anyway under some circumstances, possibly Windows reparse points
-// or something. Hence the "skip" return from this one.
-func (w *walker) walkSymlink(absPath, relPath string, dchan chan protocol.FileInfo) (skip bool, err error) {
+// walkSymlink returns nil or an error, if the error is of the nature that
+// it should stop the entire walk.
+func (w *walker) walkSymlink(absPath, relPath string, dchan chan protocol.FileInfo) error {
 	// If the target is a directory, do NOT descend down there. This will
 	// cause files to get tracked, and removing the symlink will as a result
 	// remove files in their real location.
 	if !symlinks.Supported {
-		return true, nil
+		return nil
 	}
 
 	// We always rehash symlinks as they have no modtime or
@@ -397,7 +394,7 @@ func (w *walker) walkSymlink(absPath, relPath string, dchan chan protocol.FileIn
 	target, targetType, err := symlinks.Read(absPath)
 	if err != nil {
 		l.Debugln("readlink error:", absPath, err)
-		return true, nil
+		return nil
 	}
 
 	// A symlink is "unchanged", if
@@ -409,7 +406,7 @@ func (w *walker) walkSymlink(absPath, relPath string, dchan chan protocol.FileIn
 	//  - the target was the same
 	cf, ok := w.CurrentFiler.CurrentFile(relPath)
 	if ok && !cf.IsDeleted() && cf.IsSymlink() && !cf.IsInvalid() && SymlinkTypeEqual(targetType, cf) && cf.SymlinkTarget == target {
-		return true, nil
+		return nil
 	}
 
 	f := protocol.FileInfo{
@@ -425,10 +422,10 @@ func (w *walker) walkSymlink(absPath, relPath string, dchan chan protocol.FileIn
 	select {
 	case dchan <- f:
 	case <-w.Cancel:
-		return false, errors.New("cancelled")
+		return errors.New("cancelled")
 	}
 
-	return false, nil
+	return nil
 }
 
 // normalizePath returns the normalized relative path (possibly after fixing