From 8b77a5b7aedb1168707f486ed540edf3e5de8819 Mon Sep 17 00:00:00 2001 From: lalyos Date: Thu, 15 May 2014 23:52:36 +0200 Subject: [PATCH 1/3] Adding test case for symlink causes infinit loop, reproduces: dotcloud#5370 normally symlinks are created as either ln -s /path/existing /path/new-name or cd /path && ln -s ./existing new-name but one can create it this way cd /path && ln -s existing new-name this drives FollowSymlinkInScope into infinite loop Docker-DCO-1.1-Signed-off-by: Lajos Papp (github: lalyos) --- pkg/symlink/fs_test.go | 13 +++++++++++++ pkg/symlink/testdata/fs/i | 1 + 2 files changed, 14 insertions(+) create mode 120000 pkg/symlink/testdata/fs/i diff --git a/pkg/symlink/fs_test.go b/pkg/symlink/fs_test.go index 1f12aa3a60..d85fd6da74 100644 --- a/pkg/symlink/fs_test.go +++ b/pkg/symlink/fs_test.go @@ -28,6 +28,19 @@ func TestFollowSymLinkNormal(t *testing.T) { } } +func TestFollowSymLinkRelativePath(t *testing.T) { + link := "testdata/fs/i" + + rewrite, err := FollowSymlinkInScope(link, "testdata") + if err != nil { + t.Fatal(err) + } + + if expected := abs(t, "testdata/fs/a"); expected != rewrite { + t.Fatalf("Expected %s got %s", expected, rewrite) + } +} + func TestFollowSymLinkUnderLinkedDir(t *testing.T) { dir, err := ioutil.TempDir("", "docker-fs-test") if err != nil { diff --git a/pkg/symlink/testdata/fs/i b/pkg/symlink/testdata/fs/i new file mode 120000 index 0000000000..2e65efe2a1 --- /dev/null +++ b/pkg/symlink/testdata/fs/i @@ -0,0 +1 @@ +a \ No newline at end of file From b51c366bfc963687b8cc14df614a2fc10bad6306 Mon Sep 17 00:00:00 2001 From: lalyos Date: Fri, 16 May 2014 00:25:38 +0200 Subject: [PATCH 2/3] Defend against infinite loop when following symlinks ideally it should never reach it, but there was already multiple issues with infinite loop at following symlinks. this fixes hanging unit tests Docker-DCO-1.1-Signed-off-by: Lajos Papp (github: lalyos) --- pkg/symlink/fs.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/symlink/fs.go b/pkg/symlink/fs.go index e91d33db4b..4dcfdf360f 100644 --- a/pkg/symlink/fs.go +++ b/pkg/symlink/fs.go @@ -3,10 +3,13 @@ package symlink import ( "fmt" "os" + "path" "path/filepath" "strings" ) +const maxLoopCounter = 100 + // FollowSymlink will follow an existing link and scope it to the root // path provided. func FollowSymlinkInScope(link, root string) (string, error) { @@ -30,7 +33,14 @@ func FollowSymlinkInScope(link, root string) (string, error) { prev = filepath.Join(prev, p) prev = filepath.Clean(prev) + loopCounter := 0 for { + loopCounter++ + + if loopCounter >= maxLoopCounter { + return "", fmt.Errorf("loopCounter reached MAX: %v", loopCounter) + } + if !strings.HasPrefix(prev, root) { // Don't resolve symlinks outside of root. For example, // we don't have to check /home in the below. From ad35d522dbfac124225e27f58bf07c61a34d78b5 Mon Sep 17 00:00:00 2001 From: lalyos Date: Fri, 25 Apr 2014 05:54:20 +0200 Subject: [PATCH 3/3] Fixes 5370 infinite/maxLoopCount loop for relative symlinks use path.IsAbs() instead of checking if first char is '/' Docker-DCO-1.1-Signed-off-by: Lajos Papp (github: lalyos) --- AUTHORS | 1 + pkg/symlink/fs.go | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/AUTHORS b/AUTHORS index 014748e187..8f75ab85b3 100644 --- a/AUTHORS +++ b/AUTHORS @@ -189,6 +189,7 @@ Kimbro Staken Kiran Gangadharan Konstantin Pelykh Kyle Conroy +Lajos Papp Laurie Voss Liang-Chi Hsieh Lokesh Mandvekar diff --git a/pkg/symlink/fs.go b/pkg/symlink/fs.go index 4dcfdf360f..257491f91b 100644 --- a/pkg/symlink/fs.go +++ b/pkg/symlink/fs.go @@ -63,10 +63,9 @@ func FollowSymlinkInScope(link, root string) (string, error) { return "", err } - switch dest[0] { - case '/': + if path.IsAbs(dest) { prev = filepath.Join(root, dest) - case '.': + } else { prev, _ = filepath.Abs(prev) if prev = filepath.Clean(filepath.Join(filepath.Dir(prev), dest)); len(prev) < len(root) {