From 8f488e46a46f2782f944f08cab893312521d3005 Mon Sep 17 00:00:00 2001
From: Tibor Vass <teabee89@gmail.com>
Date: Wed, 19 Nov 2014 11:27:34 -0500
Subject: [PATCH] archive: do not call FollowSymlinkInScope in createTarFile

Signed-off-by: Tibor Vass <teabee89@gmail.com>
---
 archive/archive.go      | 15 ++++++++-------
 archive/archive_test.go | 14 ++++++++++++++
 symlink/fs.go           |  4 +---
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/archive/archive.go b/archive/archive.go
index c525501..155145f 100644
--- a/archive/archive.go
+++ b/archive/archive.go
@@ -22,7 +22,6 @@ import (
 	"github.com/docker/docker/pkg/log"
 	"github.com/docker/docker/pkg/pools"
 	"github.com/docker/docker/pkg/promise"
-	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/system"
 )
 
@@ -286,12 +285,14 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, L
 		}
 
 	case tar.TypeSymlink:
-		// check for symlink breakout
-		if _, err := symlink.FollowSymlinkInScope(filepath.Join(filepath.Dir(path), hdr.Linkname), extractDir); err != nil {
-			if _, ok := err.(symlink.ErrBreakout); ok {
-				return breakoutError(fmt.Errorf("invalid symlink %q -> %q", path, hdr.Linkname))
-			}
-			return err
+		// 	path 				-> hdr.Linkname = targetPath
+		// e.g. /extractDir/path/to/symlink 	-> ../2/file	= /extractDir/path/2/file
+		targetPath := filepath.Join(filepath.Dir(path), hdr.Linkname)
+
+		// the reason we don't need to check symlinks in the path (with FollowSymlinkInScope) is because
+		// that symlink would first have to be created, which would be caught earlier, at this very check:
+		if !strings.HasPrefix(targetPath, extractDir) {
+			return breakoutError(fmt.Errorf("invalid symlink %q -> %q", path, hdr.Linkname))
 		}
 		if err := os.Symlink(hdr.Linkname, path); err != nil {
 			return err
diff --git a/archive/archive_test.go b/archive/archive_test.go
index 76dab21..7c9db44 100644
--- a/archive/archive_test.go
+++ b/archive/archive_test.go
@@ -426,6 +426,20 @@ func TestUntarInvalidSymlink(t *testing.T) {
 				Mode:     0644,
 			},
 		},
+		{ // try writing to victim/newdir/newfile with a symlink in the path
+			{
+				// this header needs to be before the next one, or else there is an error
+				Name:     "dir/loophole",
+				Typeflag: tar.TypeSymlink,
+				Linkname: "../../victim",
+				Mode:     0755,
+			},
+			{
+				Name:     "dir/loophole/newdir/newfile",
+				Typeflag: tar.TypeReg,
+				Mode:     0644,
+			},
+		},
 	} {
 		if err := testBreakout("untar", "docker-TestUntarInvalidSymlink", headers); err != nil {
 			t.Fatalf("i=%d. %v", i, err)
diff --git a/symlink/fs.go b/symlink/fs.go
index 09271ff..6ce99c6 100644
--- a/symlink/fs.go
+++ b/symlink/fs.go
@@ -10,8 +10,6 @@ import (
 
 const maxLoopCounter = 100
 
-type ErrBreakout error
-
 // FollowSymlink will follow an existing link and scope it to the root
 // path provided.
 // The role of this function is to return an absolute path in the root
@@ -36,7 +34,7 @@ func FollowSymlinkInScope(link, root string) (string, error) {
 	}
 
 	if !strings.HasPrefix(filepath.Dir(link), root) {
-		return "", ErrBreakout(fmt.Errorf("%s is not within %s", link, root))
+		return "", fmt.Errorf("%s is not within %s", link, root)
 	}
 
 	prev := "/"