gui: Don't set default path editing existing folders without label (fixes #4297)

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4298
LGTM: calmh

gui/default/index.html

@@ -394,7 +394,7 @@
                     </tr>
                     <tr>
                       <th><span class="fa fa-fw fa-share-alt"></span>&nbsp;<span translate>Shared With</span></th>
-                      <td class="text-right">{{sharesFolder(folder)}}</td>
+                      <td class="text-right" title="{{sharesFolder(folder)}}">{{sharesFolder(folder)}}</td>
                     </tr>
                     <tr>
                       <th><span class="fa fa-fw fa-clock-o"></span>&nbsp;<span translate>Last Scan</span></th>
@@ -645,7 +645,7 @@
                     </tr>
                     <tr ng-if="deviceFolders(deviceCfg).length > 0">
                       <th><span class="fa fa-fw fa-folder"></span>&nbsp;<span translate>Folders</span></th>
-                      <td class="text-right">{{deviceFolders(deviceCfg).map(folderLabel).join(", ")}}</td>
+                      <td class="text-right" title="{{deviceFolders(deviceCfg).map(folderLabel).join(", ")}}">{{deviceFolders(deviceCfg).map(folderLabel).join(", ")}}</td>
                     </tr>
                   </tbody>
                 </table>

gui/default/syncthing/core/syncthingController.js

@@ -622,6 +622,10 @@ angular.module('syncthing.core')
             return path;
         }
 
+        function shouldSetDefaultFolderPath() {
+            return $scope.config.options && $scope.config.options.defaultFolderPath && !$scope.editingExisting && $scope.folderEditor.folderPath.$pristine
+        }
+
         $scope.neededPageChanged = function (page) {
             $scope.neededCurrentPage = page;
             refreshNeed($scope.neededFolder);
@@ -1388,14 +1392,14 @@ angular.module('syncthing.core')
         });
 
         $scope.$watch('currentFolder.label', function (newvalue) {
-            if (!$scope.config.options || !$scope.config.options.defaultFolderPath || $scope.editingExisting || !$scope.folderEditor.folderPath.$pristine || !newvalue) {
+            if (!newvalue || !shouldSetDefaultFolderPath()) {
                 return;
             }
             $scope.currentFolder.path = pathJoin($scope.config.options.defaultFolderPath, newvalue);
         });
 
         $scope.$watch('currentFolder.id', function (newvalue) {
-            if (!$scope.config.options || !$scope.config.options.defaultFolderPath || !$scope.folderEditor.folderPath.$pristine || !newvalue || $scope.currentFolder.label) {
+            if (!newvalue || !shouldSetDefaultFolderPath() || $scope.currentFolder.label) {
                 return;
             }
             $scope.currentFolder.path = pathJoin($scope.config.options.defaultFolderPath, newvalue);

lib/config/config.go

@@ -17,6 +17,8 @@ import (
 	"net/url"
 	"os"
 	"path"
+	"path/filepath"
+	"runtime"
 	"sort"
 	"strconv"
 	"strings"
@@ -29,7 +31,7 @@ import (
 
 const (
 	OldestHandledVersion = 10
-	CurrentVersion       = 20
+	CurrentVersion       = 21
 	MaxRescanIntervalS   = 365 * 24 * 60 * 60
 )
 
@@ -314,6 +316,9 @@ func (cfg *Configuration) clean() error {
 	if cfg.Version == 19 {
 		convertV19V20(cfg)
 	}
+	if cfg.Version == 20 {
+		convertV20V21(cfg)
+	}
 
 	// Build a list of available devices
 	existingDevices := make(map[protocol.DeviceID]bool)
@@ -363,6 +368,30 @@ func (cfg *Configuration) clean() error {
 	return nil
 }
 
+func convertV20V21(cfg *Configuration) {
+	for _, folder := range cfg.Folders {
+		switch folder.Versioning.Type {
+		case "simple", "trashcan":
+			// Clean out symlinks in the known place
+			cleanSymlinks(filepath.Join(folder.Path(), ".stversions"))
+		case "staggered":
+			versionDir := folder.Versioning.Params["versionsPath"]
+			if versionDir == "" {
+				// default place
+				cleanSymlinks(filepath.Join(folder.Path(), ".stversions"))
+			} else if filepath.IsAbs(versionDir) {
+				// absolute
+				cleanSymlinks(versionDir)
+			} else {
+				// relative to folder
+				cleanSymlinks(filepath.Join(folder.Path(), versionDir))
+			}
+		}
+	}
+
+	cfg.Version = 21
+}
+
 func convertV19V20(cfg *Configuration) {
 	cfg.Options.MinHomeDiskFree = Size{Value: cfg.Options.DeprecatedMinHomeDiskFreePct, Unit: "%"}
 	cfg.Options.DeprecatedMinHomeDiskFreePct = 0
@@ -640,3 +669,23 @@ loop:
 	}
 	return devices[0:count]
 }
+
+func cleanSymlinks(dir string) {
+	if runtime.GOOS == "windows" {
+		// We don't do symlinks on Windows. Additionally, there may
+		// be things that look like symlinks that are not, which we
+		// should leave alone. Deduplicated files, for example.
+		return
+	}
+	filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.Mode()&os.ModeSymlink != 0 {
+			l.Infoln("Removing incorrectly versioned symlink", path)
+			os.Remove(path)
+			return filepath.SkipDir
+		}
+		return nil
+	})
+}

lib/config/testdata/v21.xml

@@ -0,0 +1,15 @@
+<configuration version="21">
+    <folder id="test" path="testdata" type="readonly" ignorePerms="false" rescanIntervalS="600" autoNormalize="true">
+        <device id="AIR6LPZ-7K4PTTV-UXQSMUU-CPQ5YWH-OEDFIIQ-JUG777G-2YQXXR5-YD6AWQR"></device>
+        <device id="P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ2"></device>
+        <minDiskFree unit="%">1</minDiskFree>
+        <maxConflicts>-1</maxConflicts>
+        <fsync>true</fsync>
+    </folder>
+    <device id="AIR6LPZ-7K4PTTV-UXQSMUU-CPQ5YWH-OEDFIIQ-JUG777G-2YQXXR5-YD6AWQR" name="node one" compression="metadata">
+        <address>tcp://a</address>
+    </device>
+    <device id="P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ2" name="node two" compression="metadata">
+        <address>tcp://b</address>
+    </device>
+</configuration>

lib/model/model_test.go

@@ -350,6 +350,24 @@ func (f *fakeConnection) addFile(name string, flags uint32, ftype protocol.FileI
 	f.fileData[name] = data
 }
 
+func (f *fakeConnection) deleteFile(name string) {
+	f.mut.Lock()
+	defer f.mut.Unlock()
+
+	for i, fi := range f.files {
+		if fi.Name == name {
+			fi.Deleted = true
+			fi.ModifiedS = time.Now().Unix()
+			fi.Version = fi.Version.Update(f.id.Short())
+			fi.Sequence = time.Now().UnixNano()
+			fi.Blocks = nil
+
+			f.files = append(append(f.files[:i], f.files[i+1:]...), fi)
+			return
+		}
+	}
+}
+
 func (f *fakeConnection) sendIndexUpdate() {
 	f.model.IndexUpdate(f.id, f.folder, f.files)
 }

lib/model/requests_test.go

@@ -10,6 +10,7 @@ import (
 	"bytes"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"testing"
@@ -204,6 +205,86 @@ func TestRequestCreateTmpSymlink(t *testing.T) {
 	}
 }
 
+func TestRequestVersioningSymlinkAttack(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("no symlink support on Windows")
+	}
+
+	// Sets up a folder with trashcan versioning and tries to use a
+	// deleted symlink to escape
+
+	cfg := defaultConfig.RawCopy()
+	cfg.Folders[0] = config.NewFolderConfiguration("default", "_tmpfolder")
+	cfg.Folders[0].PullerSleepS = 1
+	cfg.Folders[0].Devices = []config.FolderDeviceConfiguration{
+		{DeviceID: device1},
+		{DeviceID: device2},
+	}
+	cfg.Folders[0].Versioning = config.VersioningConfiguration{
+		Type: "trashcan",
+	}
+	w := config.Wrap("/tmp/cfg", cfg)
+
+	db := db.OpenMemory()
+	m := NewModel(w, device1, "syncthing", "dev", db, nil)
+	m.AddFolder(cfg.Folders[0])
+	m.ServeBackground()
+	m.StartFolder("default")
+	defer m.Stop()
+
+	defer os.RemoveAll("_tmpfolder")
+
+	fc := addFakeConn(m, device2)
+	fc.folder = "default"
+
+	// Create a temporary directory that we will use as target to see if
+	// we can escape to it
+	tmpdir, err := ioutil.TempDir("", "syncthing-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// We listen for incoming index updates and trigger when we see one for
+	// the expected test file.
+	idx := make(chan int)
+	fc.mut.Lock()
+	fc.indexFn = func(folder string, fs []protocol.FileInfo) {
+		idx <- len(fs)
+	}
+	fc.mut.Unlock()
+
+	// Send an update for the test file, wait for it to sync and be reported back.
+	fc.addFile("foo", 0644, protocol.FileInfoTypeSymlink, []byte(tmpdir))
+	fc.sendIndexUpdate()
+
+	for updates := 0; updates < 1; updates += <-idx {
+	}
+
+	// Delete the symlink, hoping for it to get versioned
+	fc.deleteFile("foo")
+	fc.sendIndexUpdate()
+	for updates := 0; updates < 1; updates += <-idx {
+	}
+
+	// Recreate foo and a file in it with some data
+	fc.addFile("foo", 0755, protocol.FileInfoTypeDirectory, nil)
+	fc.addFile("foo/test", 0644, protocol.FileInfoTypeFile, []byte("testtesttest"))
+	fc.sendIndexUpdate()
+	for updates := 0; updates < 1; updates += <-idx {
+	}
+
+	// Remove the test file and see if it escaped
+	fc.deleteFile("foo/test")
+	fc.sendIndexUpdate()
+	for updates := 0; updates < 1; updates += <-idx {
+	}
+
+	path := filepath.Join(tmpdir, "test")
+	if _, err := os.Lstat(path); !os.IsNotExist(err) {
+		t.Fatal("File escaped to", path)
+	}
+}
+
 func setupModelWithConnection() (*Model, *fakeConnection) {
 	cfg := defaultConfig.RawCopy()
 	cfg.Folders[0] = config.NewFolderConfiguration("default", "_tmpfolder")

lib/model/rwfolder.go

@@ -855,7 +855,7 @@ func (f *sendReceiveFolder) deleteFile(file protocol.FileInfo) {
 		err = osutil.InWritableDir(func(name string) error {
 			return f.moveForConflict(name, file.ModifiedBy.String())
 		}, realName)
-	} else if f.versioner != nil {
+	} else if f.versioner != nil && !cur.IsSymlink() {
 		err = osutil.InWritableDir(f.versioner.Archive, realName)
 	} else {
 		err = osutil.InWritableDir(os.Remove, realName)
@@ -1463,7 +1463,7 @@ func (f *sendReceiveFolder) performFinish(state *sharedPullerState) error {
 				return err
 			}
 
-		case f.versioner != nil:
+		case f.versioner != nil && !state.file.IsSymlink():
 			// If we should use versioning, let the versioner archive the old
 			// file before we replace it. Archiving a non-existent file is not
 			// an error.

lib/versioner/external.go

@@ -41,13 +41,16 @@ func NewExternal(folderID, folderPath string, params map[string]string) Versione
 // Archive moves the named file away to a version archive. If this function
 // returns nil, the named file does not exist any more (has been archived).
 func (v External) Archive(filePath string) error {
-	_, err := osutil.Lstat(filePath)
+	info, err := osutil.Lstat(filePath)
 	if os.IsNotExist(err) {
 		l.Debugln("not archiving nonexistent file", filePath)
 		return nil
 	} else if err != nil {
 		return err
 	}
+	if info.Mode()&os.ModeSymlink != 0 {
+		panic("bug: attempting to version a symlink")
+	}
 
 	l.Debugln("archiving", filePath)
 

lib/versioner/simple.go

@@ -50,6 +50,9 @@ func (v Simple) Archive(filePath string) error {
 	} else if err != nil {
 		return err
 	}
+	if fileInfo.Mode()&os.ModeSymlink != 0 {
+		panic("bug: attempting to version a symlink")
+	}
 
 	versionsDir := filepath.Join(v.folderPath, ".stversions")
 	_, err = os.Stat(versionsDir)

lib/versioner/staggered.go

@@ -244,13 +244,16 @@ func (v *Staggered) Archive(filePath string) error {
 	v.mutex.Lock()
 	defer v.mutex.Unlock()
 
-	_, err := osutil.Lstat(filePath)
+	info, err := osutil.Lstat(filePath)
 	if os.IsNotExist(err) {
 		l.Debugln("not archiving nonexistent file", filePath)
 		return nil
 	} else if err != nil {
 		return err
 	}
+	if info.Mode()&os.ModeSymlink != 0 {
+		panic("bug: attempting to version a symlink")
+	}
 
 	if _, err := os.Stat(v.versionsPath); err != nil {
 		if os.IsNotExist(err) {

lib/versioner/trashcan.go

@@ -44,13 +44,16 @@ func NewTrashcan(folderID, folderPath string, params map[string]string) Versione
 // Archive moves the named file away to a version archive. If this function
 // returns nil, the named file does not exist any more (has been archived).
 func (t *Trashcan) Archive(filePath string) error {
-	_, err := osutil.Lstat(filePath)
+	info, err := osutil.Lstat(filePath)
 	if os.IsNotExist(err) {
 		l.Debugln("not archiving nonexistent file", filePath)
 		return nil
 	} else if err != nil {
 		return err
 	}
+	if info.Mode()&os.ModeSymlink != 0 {
+		panic("bug: attempting to version a symlink")
+	}
 
 	versionsDir := filepath.Join(t.folderPath, ".stversions")
 	if _, err := os.Stat(versionsDir); err != nil {