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
This commit is contained in:
Jakob Borg 2017-01-05 13:26:29 +00:00 committed by Audrius Butkevicius
parent 29d010ec0e
commit 926c88cfc4

View File

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