octoleo/syncthing
1
0
Fork 0
mirror of https://github.com/octoleo/syncthing.git synced 2024-12-22 02:48:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

lib/scanner, lib/fs: Don't create file infos with abs paths (fixes #4799) (#4800)

Browse Source
This commit is contained in:
Simon Frei 2018-03-12 13:18:59 +01:00
parent 7a92f6c6b1
commit bf165d680f
11 changed files with 214 additions and 153 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
cmd/syncthing/main.go
View File

@ -752,21 +752,9 @@ func syncthingMain(runtimeOptions RuntimeOptions) {
// have been incorrectly ignore filtered.
ldb.DropDeltaIndexIDs()
}
if cfg.RawCopy().OriginalVersion < 19 {
// Converts old symlink types to new in the entire database.
ldb.ConvertSymlinkTypes()
}
if cfg.RawCopy().OriginalVersion < 26 {
// Adds invalid (ignored) files to global list of files
changed := 0
for folderID, folderCfg := range folders {
changed += ldb.AddInvalidToGlobal([]byte(folderID), protocol.LocalDeviceID[:])
for _, deviceCfg := range folderCfg.Devices {
changed += ldb.AddInvalidToGlobal([]byte(folderID), deviceCfg.DeviceID[:])
}
}
l.Infof("Database update: Added %d ignored files to the global list", changed)
}
// Potential database transitions
ldb.UpdateSchema()
m := model.NewModel(cfg, myID, "syncthing", Version, ldb, protectedFiles)

3
lib/db/leveldb.go
View File

@ -15,6 +15,8 @@ import (
"github.com/syndtr/goleveldb/leveldb/opt"
)
const dbVersion = 1
const (
KeyTypeDevice = iota
KeyTypeGlobal
@ -26,6 +28,7 @@ const (
KeyTypeDeviceIdx
KeyTypeIndexID
KeyTypeFolderMeta
KeyTypeMiscData
)
func (l VersionList) String() string {

146
lib/db/leveldb_dbinstance.go
View File

@ -83,6 +83,20 @@ func newDBInstance(db *leveldb.DB, location string) *Instance {
return i
}
// UpdateSchema does transitions to the current db version if necessary
func (db *Instance) UpdateSchema() {
miscDB := NewNamespacedKV(db, string(KeyTypeMiscData))
prevVersion, _ := miscDB.Int64("dbVersion")
if prevVersion == 0 {
db.updateSchema0to1()
}
if prevVersion != dbVersion {
miscDB.PutInt64("dbVersion", dbVersion)
}
}
// Committed returns the number of items committed to the database since startup
func (db *Instance) Committed() int64 {
return atomic.LoadInt64(&db.committed)
@ -531,21 +545,39 @@ func (db *Instance) checkGlobals(folder []byte, meta *metadataTracker) {
l.Debugf("db check completed for %q", folder)
}
// ConvertSymlinkTypes should be run once only on an old database. It
// changes SYMLINK_FILE and SYMLINK_DIRECTORY types to the current SYMLINK
// type (previously SYMLINK_UNKNOWN). It does this for all devices, both
// local and remote, and does not reset delta indexes. It shouldn't really
// matter what the symlink type is, but this cleans it up for a possible
// future when SYMLINK_FILE and SYMLINK_DIRECTORY are no longer understood.
func (db *Instance) ConvertSymlinkTypes() {
func (db *Instance) updateSchema0to1() {
t := db.newReadWriteTransaction()
defer t.close()
dbi := t.NewIterator(util.BytesPrefix([]byte{KeyTypeDevice}), nil)
defer dbi.Release()
conv := 0
symlinkConv := 0
changedFolders := make(map[string]struct{})
ignAdded := 0
meta := newMetadataTracker() // dummy metadata tracker
for dbi.Next() {
folder := db.deviceKeyFolder(dbi.Key())
device := db.deviceKeyDevice(dbi.Key())
name := string(db.deviceKeyName(dbi.Key()))
// Remove files with absolute path (see #4799)
if strings.HasPrefix(name, "/") {
if _, ok := changedFolders[string(folder)]; !ok {
changedFolders[string(folder)] = struct{}{}
}
t.removeFromGlobal(folder, device, nil, nil)
t.Delete(dbi.Key())
t.checkFlush()
continue
}
// Change SYMLINK_FILE and SYMLINK_DIRECTORY types to the current SYMLINK
// type (previously SYMLINK_UNKNOWN). It does this for all devices, both
// local and remote, and does not reset delta indexes. It shouldn't really
// matter what the symlink type is, but this cleans it up for a possible
// future when SYMLINK_FILE and SYMLINK_DIRECTORY are no longer understood.
var f protocol.FileInfo
if err := f.Unmarshal(dbi.Value()); err != nil {
// probably can't happen
@ -559,99 +591,25 @@ func (db *Instance) ConvertSymlinkTypes() {
}
t.Put(dbi.Key(), bs)
t.checkFlush()
conv++
symlinkConv++
}
}
l.Infof("Updated symlink type for %d index entries", conv)
}
// AddInvalidToGlobal searches for invalid files and adds them to the global list.
// Invalid files exist in the db if they once were not ignored and subsequently
// ignored. In the new system this is still valid, but invalid files must also be
// in the global list such that they cannot be mistaken for missing files.
func (db *Instance) AddInvalidToGlobal(folder, device []byte) int {
t := db.newReadWriteTransaction()
defer t.close()
dbi := t.NewIterator(util.BytesPrefix(db.deviceKey(folder, device, nil)[:keyPrefixLen+keyFolderLen+keyDeviceLen]), nil)
defer dbi.Release()
changed := 0
for dbi.Next() {
var file protocol.FileInfo
if err := file.Unmarshal(dbi.Value()); err != nil {
// probably can't happen
continue
}
if file.Invalid {
changed++
l.Debugf("add invalid to global; folder=%q device=%v file=%q version=%v", folder, protocol.DeviceIDFromBytes(device), file.Name, file.Version)
// this is an adapted version of readWriteTransaction.updateGlobal
name := []byte(file.Name)
gk := t.db.globalKey(folder, name)
var fl VersionList
if svl, err := t.Get(gk, nil); err == nil {
fl.Unmarshal(svl) // skip error, range handles success case
}
nv := FileVersion{
Device: device,
Version: file.Version,
Invalid: file.Invalid,
}
inserted := false
// Find a position in the list to insert this file. The file at the front
// of the list is the newer, the "global".
insert:
for i := range fl.Versions {
switch fl.Versions[i].Version.Compare(file.Version) {
case protocol.Equal:
// Invalid files should go after a valid file of equal version
if nv.Invalid {
continue insert
}
fallthrough
case protocol.Lesser:
// The version at this point in the list is equal to or lesser
// ("older") than us. We insert ourselves in front of it.
fl.Versions = insertVersion(fl.Versions, i, nv)
inserted = true
break insert
case protocol.ConcurrentLesser, protocol.ConcurrentGreater:
// The version at this point is in conflict with us. We must pull
// the actual file metadata to determine who wins. If we win, we
// insert ourselves in front of the loser here. (The "Lesser" and
// "Greater" in the condition above is just based on the device
// IDs in the version vector, which is not the only thing we use
// to determine the winner.)
//
// A surprise missing file entry here is counted as a win for us.
of, ok := t.getFile(folder, fl.Versions[i].Device, name)
if !ok || file.WinsConflict(of) {
fl.Versions = insertVersion(fl.Versions, i, nv)
inserted = true
break insert
}
// Add invalid files to global list
if f.Invalid {
if t.updateGlobal(folder, device, f, meta) {
if _, ok := changedFolders[string(folder)]; !ok {
changedFolders[string(folder)] = struct{}{}
}
ignAdded++
}
if !inserted {
// We didn't find a position for an insert above, so append to the end.
fl.Versions = append(fl.Versions, nv)
}
t.Put(gk, mustMarshal(&fl))
}
}
return changed
for folder := range changedFolders {
db.dropFolderMeta([]byte(folder))
}
l.Infof("Updated symlink type for %d index entries and added %d invalid files to global list", symlinkConv, ignAdded)
}
// deviceKey returns a byte slice encoding the following information:

26
lib/fs/basicfs.go
View File

@ -88,28 +88,10 @@ func (f *BasicFilesystem) rooted(rel string) (string, error) {
expectedPrefix += pathSep
}
// The relative path should be clean from internal dotdots and similar
// funkyness.
rel = filepath.FromSlash(rel)
if filepath.Clean(rel) != rel {
return "", ErrInvalidFilename
}
// It is not acceptable to attempt to traverse upwards.
switch rel {
case "..", pathSep:
return "", ErrNotRelative
}
if strings.HasPrefix(rel, ".."+pathSep) {
return "", ErrNotRelative
}
if strings.HasPrefix(rel, pathSep+pathSep) {
// The relative path may pretend to be an absolute path within the
// root, but the double path separator on Windows implies something
// else. It would get cleaned by the Join below, but it's out of
// spec anyway.
return "", ErrNotRelative
var err error
rel, err = Canonicalize(rel)
if err != nil {
return "", err
}
// The supposedly correct path is the one filepath.Join will return, as

37
lib/fs/basicfs_test.go
View File

@ -364,17 +364,17 @@ func TestRooted(t *testing.T) {
{"baz/foo/", "bar/baz", "baz/foo/bar/baz", true},
{"baz/foo/", "/bar/baz", "baz/foo/bar/baz", true},
// Not escape attempts, but oddly formatted relative paths. Disallowed.
{"foo", "./bar", "", false},
{"baz/foo", "./bar", "", false},
{"foo", "./bar/baz", "", false},
{"baz/foo", "./bar/baz", "", false},
{"baz/foo", "bar/../baz", "", false},
{"baz/foo", "/bar/../baz", "", false},
{"baz/foo", "./bar/../baz", "", false},
{"baz/foo", "bar/../baz", "", false},
{"baz/foo", "/bar/../baz", "", false},
{"baz/foo", "./bar/../baz", "", false},
// Not escape attempts, but oddly formatted relative paths.
{"foo", "", "foo/", true},
{"foo", "/", "foo/", true},
{"foo", "/..", "foo/", true},
{"foo", "./bar", "foo/bar", true},
{"baz/foo", "./bar", "baz/foo/bar", true},
{"foo", "./bar/baz", "foo/bar/baz", true},
{"baz/foo", "./bar/baz", "baz/foo/bar/baz", true},
{"baz/foo", "bar/../baz", "baz/foo/baz", true},
{"baz/foo", "/bar/../baz", "baz/foo/baz", true},
{"baz/foo", "./bar/../baz", "baz/foo/baz", true},
// Results in an allowed path, but does it by probing. Disallowed.
{"foo", "../foo", "", false},
@ -385,10 +385,7 @@ func TestRooted(t *testing.T) {
{"baz/foo", "bar/../../../baz/foo/bar", "", false},
// Escape attempts.
{"foo", "", "", false},
{"foo", "/", "", false},
{"foo", "..", "", false},
{"foo", "/..", "", false},
{"foo", "../", "", false},
{"foo", "../bar", "", false},
{"foo", "../foobar", "", false},
@ -413,8 +410,8 @@ func TestRooted(t *testing.T) {
{"/", "/foo", "/foo", true},
{"/", "../foo", "", false},
{"/", "..", "", false},
{"/", "/", "", false},
{"/", "", "", false},
{"/", "/", "/", true},
{"/", "", "/", true},
// special case for filesystems to be able to MkdirAll('.') for example
{"/", ".", "/", true},
@ -427,11 +424,11 @@ func TestRooted(t *testing.T) {
{`c:\`, `\foo`, `c:\foo`, true},
{`\\?\c:\`, `\foo`, `\\?\c:\foo`, true},
{`c:\`, `\\foo`, ``, false},
{`c:\`, ``, ``, false},
{`c:\`, `\`, ``, false},
{`c:\`, ``, `c:\`, true},
{`c:\`, `\`, `c:\`, true},
{`\\?\c:\`, `\\foo`, ``, false},
{`\\?\c:\`, ``, ``, false},
{`\\?\c:\`, `\`, ``, false},
{`\\?\c:\`, ``, `\\?\c:\`, true},
{`\\?\c:\`, `\`, `\\?\c:\`, true},
// makes no sense, but will be treated simply as a bad filename
{`c:\foo`, `d:\bar`, `c:\foo\d:\bar`, true},

36
lib/fs/filesystem.go
View File

@ -198,3 +198,39 @@ func IsInternal(file string) bool {
}
return false
}
// Canonicalize checks that the file path is valid and returns it in the "canonical" form:
// - /foo/bar -> foo/bar
// - / -> "."
func Canonicalize(file string) (string, error) {
pathSep := string(PathSeparator)
if strings.HasPrefix(file, pathSep+pathSep) {
// The relative path may pretend to be an absolute path within
// the root, but the double path separator on Windows implies
// something else and is out of spec.
return "", ErrNotRelative
}
// The relative path should be clean from internal dotdots and similar
// funkyness.
file = filepath.Clean(file)
// It is not acceptable to attempt to traverse upwards.
switch file {
case "..":
return "", ErrNotRelative
}
if strings.HasPrefix(file, ".."+pathSep) {
return "", ErrNotRelative
}
if strings.HasPrefix(file, pathSep) {
if file == pathSep {
return ".", nil
}
return file[1:], nil
}
return file, nil
}

57
lib/fs/filesystem_test.go
View File

@ -41,3 +41,60 @@ func TestIsInternal(t *testing.T) {
}
}
}
func TestCanonicalize(t *testing.T) {
type testcase struct {
path string
expected string
ok bool
}
cases := []testcase{
// Valid cases
{"/bar", "bar", true},
{"/bar/baz", "bar/baz", true},
{"bar", "bar", true},
{"bar/baz", "bar/baz", true},
// Not escape attempts, but oddly formatted relative paths
{"", ".", true},
{"/", ".", true},
{"/..", ".", true},
{"./bar", "bar", true},
{"./bar/baz", "bar/baz", true},
{"bar/../baz", "baz", true},
{"/bar/../baz", "baz", true},
{"./bar/../baz", "baz", true},
// Results in an allowed path, but does it by probing. Disallowed.
{"../foo", "", false},
{"../foo/bar", "", false},
{"../foo/bar", "", false},
{"../../baz/foo/bar", "", false},
{"bar/../../foo/bar", "", false},
{"bar/../../../baz/foo/bar", "", false},
// Escape attempts.
{"..", "", false},
{"../", "", false},
{"../bar", "", false},
{"../foobar", "", false},
{"bar/../../quux/baz", "", false},
}
for _, tc := range cases {
res, err := Canonicalize(tc.path)
if tc.ok {
if err != nil {
t.Errorf("Unexpected error for Canonicalize(%q): %v", tc.path, err)
continue
}
exp := filepath.FromSlash(tc.expected)
if res != exp {
t.Errorf("Unexpected result for Canonicalize(%q): %q != expected %q", tc.path, res, exp)
}
} else if err == nil {
t.Errorf("Unexpected pass for Canonicalize(%q) => %q", tc.path, res)
continue
}
}
}

7
lib/fs/walkfs.go
View File

@ -38,7 +38,12 @@ func NewWalkFilesystem(next Filesystem) Filesystem {
// walk recursively descends path, calling walkFn.
func (f *walkFilesystem) walk(path string, info FileInfo, walkFn WalkFunc) error {
err := walkFn(path, info, nil)
path, err := Canonicalize(path)
if err != nil {
return err
}
err = walkFn(path, info, nil)
if err != nil {
if info.IsDir() && err == SkipDir {
return nil

7
lib/model/model.go
View File

@ -2777,7 +2777,12 @@ func unifySubs(dirs []string, exists func(dir string) bool) []string {
}
prev := "./" // Anything that can't be parent of a clean path
for i := 0; i < len(dirs); {
dir := filepath.Clean(dirs[i])
dir, err := fs.Canonicalize(dirs[i])
if err != nil {
l.Debugf("Skipping %v for scan: %s", dirs[i], err)
dirs = append(dirs[:i], dirs[i+1:]...)
continue
}
if dir == prev || strings.HasPrefix(dir, prev+string(fs.PathSeparator)) {
dirs = append(dirs[:i], dirs[i+1:]...)
continue

6
lib/model/model_test.go
View File

@ -2159,6 +2159,12 @@ func unifySubsCases() []unifySubsCase {
[]string{"foo"},
nil,
},
{
// 10. absolute path
[]string{"/foo"},
[]string{"foo"},
[]string{"foo"},
},
}
if runtime.GOOS == "windows" {

24
lib/scanner/walk_test.go
View File

@ -498,6 +498,30 @@ func TestStopWalk(t *testing.T) {
}
}
func TestIssue4799(t *testing.T) {
tmp, err := ioutil.TempDir("", "")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(tmp)
fs := fs.NewFilesystem(fs.FilesystemTypeBasic, tmp)
fd, err := fs.Create("foo")
if err != nil {
t.Fatal(err)
}
fd.Close()
files, err := walkDir(fs, "/foo")
if err != nil {
t.Fatal(err)
}
if len(files) != 1 || files[0].Name != "foo" {
t.Error(`Received unexpected file infos when walking "/foo"`, files)
}
}
// Verify returns nil or an error describing the mismatch between the block
// list and actual reader contents
func verify(r io.Reader, blocksize int, blocks []protocol.BlockInfo) error {