archive: do not call FollowSymlinkInScope in createTarFile

Signed-off-by: Tibor Vass <teabee89@gmail.com>
This commit is contained in:
Tibor Vass 2014-11-19 11:27:34 -05:00
parent cdf5a07698
commit 8f488e46a4
3 changed files with 23 additions and 10 deletions

View file

@ -22,7 +22,6 @@ import (
"github.com/docker/docker/pkg/log" "github.com/docker/docker/pkg/log"
"github.com/docker/docker/pkg/pools" "github.com/docker/docker/pkg/pools"
"github.com/docker/docker/pkg/promise" "github.com/docker/docker/pkg/promise"
"github.com/docker/docker/pkg/symlink"
"github.com/docker/docker/pkg/system" "github.com/docker/docker/pkg/system"
) )
@ -286,13 +285,15 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, L
} }
case tar.TypeSymlink: case tar.TypeSymlink:
// check for symlink breakout // path -> hdr.Linkname = targetPath
if _, err := symlink.FollowSymlinkInScope(filepath.Join(filepath.Dir(path), hdr.Linkname), extractDir); err != nil { // e.g. /extractDir/path/to/symlink -> ../2/file = /extractDir/path/2/file
if _, ok := err.(symlink.ErrBreakout); ok { 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)) return breakoutError(fmt.Errorf("invalid symlink %q -> %q", path, hdr.Linkname))
} }
return err
}
if err := os.Symlink(hdr.Linkname, path); err != nil { if err := os.Symlink(hdr.Linkname, path); err != nil {
return err return err
} }

View file

@ -426,6 +426,20 @@ func TestUntarInvalidSymlink(t *testing.T) {
Mode: 0644, 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 { if err := testBreakout("untar", "docker-TestUntarInvalidSymlink", headers); err != nil {
t.Fatalf("i=%d. %v", i, err) t.Fatalf("i=%d. %v", i, err)

View file

@ -10,8 +10,6 @@ import (
const maxLoopCounter = 100 const maxLoopCounter = 100
type ErrBreakout error
// FollowSymlink will follow an existing link and scope it to the root // FollowSymlink will follow an existing link and scope it to the root
// path provided. // path provided.
// The role of this function is to return an absolute path in the root // 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) { 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 := "/" prev := "/"