From 49ea5b8cf527eb021f46d8ac3ff434debb52ede6 Mon Sep 17 00:00:00 2001 From: Rafal Jeczalik Date: Wed, 24 Dec 2014 10:44:37 +0100 Subject: [PATCH] FSEvents: limit iteration count for circular() To poor-man-protect from circular symlinks. --- util.go | 9 ++++ watcher_fsevents.go | 53 ++++++++++---------- watcher_fsevents_test.go | 102 +++++++++++++++++++++++++++++++++++---- 3 files changed, 130 insertions(+), 34 deletions(-) diff --git a/util.go b/util.go index 343a881..8c3de02 100644 --- a/util.go +++ b/util.go @@ -4,6 +4,15 @@ import "os" const sep = string(os.PathSeparator) +func nonil(err ...error) error { + for _, err := range err { + if err != nil { + return err + } + } + return nil +} + // Joinevents TODO func joinevents(events []Event) (e Event) { if len(events) == 0 { diff --git a/watcher_fsevents.go b/watcher_fsevents.go index 423f68b..96a7c6f 100644 --- a/watcher_fsevents.go +++ b/watcher_fsevents.go @@ -16,6 +16,34 @@ var ( errNotWatched = errors.New("path is not being watched") ) +var errDepth = errors.New("exceeded allowed iteration count (circular symlink?)") + +func canonical(p string) (string, error) { + for i, depth := 1, 1; i < len(p); i, depth = i+1, depth+1 { + if depth > 128 { + return "", &os.PathError{Op: "canonical", Path: p, Err: errDepth} + } + if j := IndexSep(p[i:]); j == -1 { + i = len(p) + } else { + i = i + j + } + fi, err := os.Lstat(p[:i]) + if err != nil { + return "", err + } + if fi.Mode()&os.ModeSymlink == os.ModeSymlink { + s, err := os.Readlink(p[:i]) + if err != nil { + return "", err + } + p = "/" + s + p[i:] + i = 1 // no guarantee s is canonical, start all over + } + } + return filepath.Clean(p), nil +} + type watch struct { c chan<- EventInfo stream *Stream @@ -62,31 +90,6 @@ func newWatcher() Watcher { } } -// TODO(rjeczalik): Detect and handle loops. -func canonical(p string) (string, error) { - for i := 1; i < len(p); i++ { - if j := IndexSep(p[i:]); j == -1 { - i = len(p) - } else { - i = i + j - } - fi, err := os.Lstat(p[:i]) - if err != nil { - return "", err - } - if fi.Mode()&os.ModeSymlink == os.ModeSymlink { - s, err := os.Readlink(p[:i]) - if err != nil { - return "", err - } - p = "/" + s + p[i:] - i = len(s) - continue - } - } - return filepath.Clean(p), nil -} - func (fse *fsevents) watch(path string, event Event, isrec int32) (err error) { if path, err = canonical(path); err != nil { return diff --git a/watcher_fsevents_test.go b/watcher_fsevents_test.go index 71d9585..464b389 100644 --- a/watcher_fsevents_test.go +++ b/watcher_fsevents_test.go @@ -3,17 +3,46 @@ package notify -import "testing" +import ( + "io/ioutil" + "os" + "testing" +) -func TestCanonicalize(t *testing.T) { - cases := [...]struct { - path string - full string - }{ - {"/etc", "/private/etc"}, - {"/tmp", "/private/tmp"}, - {"/var", "/private/var"}, +func tmpfile(s string) (string, error) { + f, err := ioutil.TempFile("/tmp", s) + if err != nil { + return "", err } + if err = nonil(f.Sync(), f.Close()); err != nil { + return "", err + } + return f.Name(), nil +} + +func symlink(s string) (string, error) { + name, err := tmpfile("symlink") + if err != nil { + return "", err + } + if err = nonil(os.Remove(name), os.Symlink(s, name)); err != nil { + return "", err + } + return name, nil +} + +func remove(s ...string) { + for _, s := range s { + os.Remove(s) + } +} + +type caseCanonical struct { + path string + full string +} + +func testCanonical(t *testing.T, cases []caseCanonical) { for i, cas := range cases { full, err := canonical(cas.path) if err != nil { @@ -26,3 +55,58 @@ func TestCanonicalize(t *testing.T) { } } } + +func TestCanonicalize(t *testing.T) { + cases := [...]caseCanonical{ + {"/etc", "/private/etc"}, + {"/etc/defaults", "/private/etc/defaults"}, + {"/etc/hosts", "/private/etc/hosts"}, + {"/tmp", "/private/tmp"}, + {"/var", "/private/var"}, + } + testCanonical(t, cases[:]) +} + +func TestCanonicalizeMultiple(t *testing.T) { + link1, err := symlink("/etc") + if err != nil { + t.Fatal(err) + } + link2, err := symlink("/tmp") + if err != nil { + t.Fatal(nonil(err, os.Remove(link1))) + } + defer remove(link1, link2) + cases := [...]caseCanonical{ + {link1, "/private/etc"}, + {link1 + "/hosts", "/private/etc/hosts"}, + {link2, "/private/tmp"}, + } + testCanonical(t, cases[:]) +} + +func TestCanonicalizeCircular(t *testing.T) { + tmp1, err := tmpfile("circular") + if err != nil { + t.Fatal(err) + } + tmp2, err := tmpfile("circular") + if err != nil { + t.Fatal(nonil(err, os.Remove(tmp1))) + } + defer remove(tmp1, tmp2) + // Symlink tmp1 -> tmp2. + if err = nonil(os.Remove(tmp1), os.Symlink(tmp2, tmp1)); err != nil { + t.Fatal(err) + } + // Symlnik tmp2 -> tmp1. + if err = nonil(os.Remove(tmp2), os.Symlink(tmp1, tmp2)); err != nil { + t.Fatal(err) + } + if _, err = canonical(tmp1); err == nil { + t.Fatalf("want canonical(%q)!=nil", tmp1) + } + if _, ok := err.(*os.PathError); !ok { + t.Fatalf("want canonical(%q)=os.PathError; got %T", tmp1, err) + } +}