archive: do not call FollowSymlinkInScope in createTarFile

Signed-off-by: Tibor Vass <[email protected]>

pkg/archive/archive.go

@@ -22,7 +22,6 @@ import (
 	"github.com/docker/docker/pkg/fileutils"
 	"github.com/docker/docker/pkg/pools"
 	"github.com/docker/docker/pkg/promise"
-	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/system"
 )
 
@@ -303,12 +302,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

pkg/archive/archive_test.go

@@ -587,6 +587,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)

pkg/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 := "/"