lib/model, lib/versioner: Prevent symlink attack via versioning (fixes #4286)

Prior to this, the following is possible:

- Create a symlink "foo -> /somewhere", it gets synced
- Delete "foo", it gets versioned
- Create "foo/bar", it gets synced
- Delete "foo/bar", it gets versioned in "/somewhere/bar"

With this change, versioners should never version symlinks.

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,82 @@ func TestRequestCreateTmpSymlink(t *testing.T) {
 	}
 }
 
+func TestRequestVersioningSymlinkAttack(t *testing.T) {
+	// 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.