github/containerd
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

Update tar path resolution

Browse Source 
Fixes bug for resolving symlinks which allowed fully resolving
an existing symlink to a path, causing some symlinks to get
overridden as symlinks to self.
Updates logic to split name into parent path, resolve the parent
path, then safely join back with the base name.
Uses the split code to ensure parent directories are created in
all cases.
Replaces `rootJoin` with filepath.Join to the root, which already
correctly cleans relative symlinks to the root.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
This commit is contained in:
Derek McGowan 2017-07-24 17:51:47 -07:00
parent 9b53b8b68d
commit afec478beb
3 changed files with 291 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
archive/path.go
View File

@ -28,7 +28,7 @@ func rootPath(root, path string) (string, error) {
}
path = newpath
if i == linksWalked {
newpath = rootJoin(newpath)
newpath = filepath.Join("/", newpath)
if path == newpath {
return filepath.Join(root, newpath), nil
}
@ -37,28 +37,12 @@ func rootPath(root, path string) (string, error) {
}
}
// rootJoin joins a path with root, cleaning up any links that
// reference above root.
func rootJoin(path string) string {
if filepath.IsAbs(path) {
path = filepath.Clean(path)
}
// Resolve any ".." or "/.." before joining to root
for !filepath.IsAbs(path) {
path = "/" + path
path = filepath.Clean(path)
}
return path
}
func walkLink(root, path string, linksWalked *int) (newpath string, islink bool, err error) {
if *linksWalked > 255 {
return "", false, errTooManyLinks
}
path = rootJoin(path)
path = filepath.Join("/", path)
if path == "/" {
return path, false, nil
}

36
archive/tar.go
View File

@ -126,19 +126,30 @@ func Apply(ctx context.Context, root string, r io.Reader) (int64, error) {
continue
}
// Note as these operations are platform specific, so must the slash be.
if !strings.HasSuffix(hdr.Name, string(os.PathSeparator)) {
// Not the root directory, ensure that the parent directory exists.
// This happened in some tests where an image had a tarfile without any
// parent directories.
parent := filepath.Dir(hdr.Name)
parentPath, err := rootPath(root, parent)
// Split name and resolve symlinks for root directory.
ppath, base := filepath.Split(hdr.Name)
ppath, err = rootPath(root, ppath)
if err != nil {
return 0, err
return 0, errors.Wrap(err, "failed to get root path")
}
// Join to root before joining to parent path to ensure relative links are
// already resolved based on the root before adding to parent.
path := filepath.Join(ppath, filepath.Join("/", base))
if path == root {
log.G(ctx).Debugf("file %q ignored: resolved to root", hdr.Name)
continue
}
// If file is not directly under root, ensure parent directory
// exists or is created.
if ppath != root {
parentPath := ppath
if base == "" {
parentPath = filepath.Dir(path)
}
if _, err := os.Lstat(parentPath); err != nil && os.IsNotExist(err) {
err = mkdirAll(parentPath, 0600)
err = mkdirAll(parentPath, 0700)
if err != nil {
return 0, err
}
@ -173,13 +184,6 @@ func Apply(ctx context.Context, root string, r io.Reader) (int64, error) {
}
}
path, err := rootPath(root, hdr.Name)
if err != nil {
return 0, errors.Wrap(err, "failed to get root path")
}
base := filepath.Base(path)
if strings.HasPrefix(base, whiteoutPrefix) {
dir := filepath.Dir(path)
if base == whiteoutOpaqueDir {

272
archive/tar_test.go
View File

@ -89,6 +89,83 @@ func TestRelativeSymlinks(t *testing.T) {
}
}
func TestSymlinks(t *testing.T) {
links := [][2]fstest.Applier{
{
fstest.Apply(
fstest.CreateDir("/bin/", 0755),
fstest.CreateFile("/bin/superbinary", []byte{0x00, 0x00}, 0755),
fstest.Symlink("../bin/superbinary", "/bin/other1"),
),
fstest.Apply(
fstest.Remove("/bin/other1"),
fstest.Symlink("/bin/superbinary", "/bin/other1"),
fstest.Symlink("../bin/superbinary", "/bin/other2"),
fstest.Symlink("superbinary", "/bin/other3"),
),
},
{
fstest.Apply(
fstest.CreateDir("/bin/", 0755),
fstest.CreateDir("/sbin/", 0755),
fstest.CreateFile("/sbin/superbinary", []byte{0x00, 0x00}, 0755),
fstest.Symlink("/sbin/superbinary", "/bin/superbinary"),
fstest.Symlink("../bin/superbinary", "/bin/other1"),
),
fstest.Apply(
fstest.Remove("/bin/other1"),
fstest.Symlink("/bin/superbinary", "/bin/other1"),
fstest.Symlink("superbinary", "/bin/other2"),
),
},
{
fstest.Apply(
fstest.CreateDir("/bin/", 0755),
fstest.CreateDir("/sbin/", 0755),
fstest.CreateFile("/sbin/superbinary", []byte{0x00, 0x00}, 0755),
fstest.Symlink("../sbin/superbinary", "/bin/superbinary"),
fstest.Symlink("../bin/superbinary", "/bin/other1"),
),
fstest.Apply(
fstest.Remove("/bin/other1"),
fstest.Symlink("/bin/superbinary", "/bin/other1"),
),
},
{
fstest.Apply(
fstest.CreateDir("/bin/", 0755),
fstest.CreateFile("/bin/actualbinary", []byte{0x00, 0x00}, 0755),
fstest.Symlink("actualbinary", "/bin/superbinary"),
fstest.Symlink("../bin/superbinary", "/bin/other1"),
fstest.Symlink("superbinary", "/bin/other2"),
),
fstest.Apply(
fstest.Remove("/bin/other1"),
fstest.Remove("/bin/other2"),
fstest.Symlink("/bin/superbinary", "/bin/other1"),
fstest.Symlink("superbinary", "/bin/other2"),
),
},
{
fstest.Apply(
fstest.CreateDir("/bin/", 0755),
fstest.CreateFile("/bin/actualbinary", []byte{0x00, 0x00}, 0755),
fstest.Symlink("actualbinary", "/bin/myapp"),
),
fstest.Apply(
fstest.Remove("/bin/myapp"),
fstest.CreateDir("/bin/myapp", 0755),
),
},
}
for i, l := range links {
if err := testDiffApply(l[0], l[1]); err != nil {
t.Fatalf("Test[%d] apply failed: %+v", i+1, err)
}
}
}
func TestBreakouts(t *testing.T) {
tc := TarContext{}.WithUidGid(os.Getuid(), os.Getgid()).WithModTime(time.Now().UTC())
expected := "unbroken"
@ -102,10 +179,17 @@ func TestBreakouts(t *testing.T) {
}
return nil
}
errFileDiff := errors.New("files differ")
sameFile := func(f1, f2 string) func(string) error {
return func(root string) error {
p1 := filepath.Join(root, f1)
p2 := filepath.Join(root, f2)
p1, err := rootPath(root, f1)
if err != nil {
return err
}
p2, err := rootPath(root, f2)
if err != nil {
return err
}
s1, err := os.Stat(p1)
if err != nil {
return err
@ -115,7 +199,20 @@ func TestBreakouts(t *testing.T) {
return err
}
if !os.SameFile(s1, s2) {
return errors.Errorf("files differ: %#v and %#v", s1, s2)
return errors.Wrapf(errFileDiff, "%#v and %#v", s1, s2)
}
return nil
}
}
notSameFile := func(f1, f2 string) func(string) error {
same := sameFile(f1, f2)
return func(root string) error {
err := same(root)
if err == nil {
return errors.New("files are the same, expected diff")
}
if errors.Cause(err) != errFileDiff {
return err
}
return nil
}
@ -124,6 +221,7 @@ func TestBreakouts(t *testing.T) {
breakouts := []struct {
name string
w WriterToTar
apply fstest.Applier
validator func(string) error
}{
{
@ -277,10 +375,103 @@ func TestBreakouts(t *testing.T) {
),
validator: sameFile("/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkParentDirectory",
w: TarAll(
tc.Dir("etc", 0770),
tc.File("/etc/passwd", []byte("inside"), 0644),
tc.Symlink("/etc/", ".."),
tc.Link("/etc/passwd", "localpasswd"),
),
validator: sameFile("/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkEmptyFilename",
w: TarAll(
tc.Dir("etc", 0770),
tc.File("/etc/passwd", []byte("inside"), 0644),
tc.Symlink("/etc/", ""),
tc.Link("/etc/passwd", "localpasswd"),
),
validator: sameFile("/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkParentRelative",
w: TarAll(
tc.Dir("etc", 0770),
tc.File("/etc/passwd", []byte("inside"), 0644),
tc.Symlink("/etc/", "localetc/sub/.."),
tc.Link("/etc/passwd", "/localetc/localpasswd"),
),
validator: sameFile("/localetc/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkSlashEnded",
w: TarAll(
tc.Dir("etc", 0770),
tc.File("/etc/passwd", []byte("inside"), 0644),
tc.Dir("localetc/", 0770),
tc.Link("/etc/passwd", "/localetc/localpasswd"),
),
validator: sameFile("/localetc/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkOverrideDirectory",
apply: fstest.Apply(
fstest.CreateDir("/etc/", 0755),
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
fstest.CreateDir("/localetc/", 0755),
),
w: TarAll(
tc.Symlink("/etc", "localetc"),
tc.Link("/etc/passwd", "/localetc/localpasswd"),
),
validator: sameFile("/localetc/localpasswd", "/etc/passwd"),
},
{
name: "SymlinkOverrideDirectoryRelative",
apply: fstest.Apply(
fstest.CreateDir("/etc/", 0755),
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
fstest.CreateDir("/localetc/", 0755),
),
w: TarAll(
tc.Symlink("../../etc", "localetc"),
tc.Link("/etc/passwd", "/localetc/localpasswd"),
),
validator: sameFile("/localetc/localpasswd", "/etc/passwd"),
},
{
name: "DirectoryOverrideSymlink",
apply: fstest.Apply(
fstest.CreateDir("/etc/", 0755),
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
fstest.Symlink("/etc", "localetc"),
),
w: TarAll(
tc.Dir("/localetc/", 0755),
tc.Link("/etc/passwd", "/localetc/localpasswd"),
),
validator: sameFile("/localetc/localpasswd", "/etc/passwd"),
},
{
name: "DirectoryOverrideSymlinkAndHardlink",
apply: fstest.Apply(
fstest.CreateDir("/etc/", 0755),
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
fstest.Symlink("etc", "localetc"),
fstest.Link("/etc/passwd", "/localetc/localpasswd"),
),
w: TarAll(
tc.Dir("/localetc/", 0755),
tc.File("/localetc/localpasswd", []byte("different"), 0644),
),
validator: notSameFile("/localetc/localpasswd", "/etc/passwd"),
},
}
for _, bo := range breakouts {
t.Run(bo.name, makeWriterToTarTest(bo.w, bo.validator))
t.Run(bo.name, makeWriterToTarTest(bo.w, bo.apply, bo.validator))
}
}
@ -288,6 +479,56 @@ func TestDiffApply(t *testing.T) {
fstest.FSSuite(t, diffApplier{})
}
func TestApplyTar(t *testing.T) {
tc := TarContext{}.WithUidGid(os.Getuid(), os.Getgid()).WithModTime(time.Now().UTC())
directoriesExist := func(dirs ...string) func(string) error {
return func(root string) error {
for _, d := range dirs {
p, err := rootPath(root, d)
if err != nil {
return err
}
if _, err := os.Stat(p); err != nil {
return errors.Wrapf(err, "failure checking existance for %v", d)
}
}
return nil
}
}
tests := []struct {
name string
w WriterToTar
apply fstest.Applier
validator func(string) error
}{
{
name: "DirectoryCreation",
apply: fstest.Apply(
fstest.CreateDir("/etc/", 0755),
),
w: TarAll(
tc.Dir("/etc/subdir", 0755),
tc.Dir("/etc/subdir2/", 0755),
tc.Dir("/etc/subdir2/more", 0755),
tc.Dir("/other/noparent-1/1", 0755),
tc.Dir("/other/noparent-2/2/", 0755),
),
validator: directoriesExist(
"etc/subdir",
"etc/subdir2",
"etc/subdir2/more",
"other/noparent-1/1",
"other/noparent-2/2",
),
},
}
for _, at := range tests {
t.Run(at.name, makeWriterToTarTest(at.w, at.apply, at.validator))
}
}
func testApply(a fstest.Applier) error {
td, err := ioutil.TempDir("", "test-apply-")
if err != nil {
@ -356,7 +597,7 @@ func testBaseDiff(a fstest.Applier) error {
return fstest.CheckDirectoryEqual(td, dest)
}
func testDiffApply(a fstest.Applier) error {
func testDiffApply(appliers ...fstest.Applier) error {
td, err := ioutil.TempDir("", "test-diff-apply-")
if err != nil {
return errors.Wrap(err, "failed to create temp dir")
@ -368,11 +609,22 @@ func testDiffApply(a fstest.Applier) error {
}
defer os.RemoveAll(dest)
for _, a := range appliers {
if err := a.Apply(td); err != nil {
return errors.Wrap(err, "failed to apply filesystem changes")
}
}
diffBytes, err := ioutil.ReadAll(Diff(context.Background(), "", td))
// Apply base changes before diff
if len(appliers) > 1 {
for _, a := range appliers[:len(appliers)-1] {
if err := a.Apply(dest); err != nil {
return errors.Wrap(err, "failed to apply base filesystem changes")
}
}
}
diffBytes, err := ioutil.ReadAll(Diff(context.Background(), dest, td))
if err != nil {
return errors.Wrap(err, "failed to create diff")
}
@ -384,7 +636,7 @@ func testDiffApply(a fstest.Applier) error {
return fstest.CheckDirectoryEqual(td, dest)
}
func makeWriterToTarTest(wt WriterToTar, validate func(string) error) func(*testing.T) {
func makeWriterToTarTest(wt WriterToTar, a fstest.Applier, validate func(string) error) func(*testing.T) {
return func(t *testing.T) {
td, err := ioutil.TempDir("", "test-writer-to-tar-")
if err != nil {
@ -392,6 +644,12 @@ func makeWriterToTarTest(wt WriterToTar, validate func(string) error) func(*test
}
defer os.RemoveAll(td)
if a != nil {
if err := a.Apply(td); err != nil {
t.Fatalf("Failed to apply filesystem to directory: %v", err)
}
}
tr := TarFromWriterTo(wt)
if _, err := Apply(context.Background(), td, tr); err != nil {