Skip to content

Commit 0a2c2a2

Browse files
stevvooestevvooe
authored
Merge pull request #2003 from dmcgowan/cherry-pick-fix-whiteout-rootpath-1.0
[release/1.0] archive: check whiteout path before removal
2 parents 1549dda + ce75ddf commit 0a2c2a2

2 files changed

Lines changed: 177 additions & 4 deletions

File tree

‎archive/tar.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ var bufferPool = &sync.Pool{
2626
},
2727
}
2828

29+
var errInvalidArchive = errors.New("invalid archive")
30+
2931
// Diff returns a tar stream of the computed filesystem
3032
// difference between the provided directories.
3133
//
@@ -220,6 +222,15 @@ func Apply(ctx context.Context, root string, r io.Reader) (int64, error) {
220222

221223
originalBase := base[len(whiteoutPrefix):]
222224
originalPath := filepath.Join(dir, originalBase)
225+
226+
// Ensure originalPath is under dir
227+
if dir[len(dir)-1] != filepath.Separator {
228+
dir += string(filepath.Separator)
229+
}
230+
if !strings.HasPrefix(originalPath, dir) {
231+
return 0, errors.Wrapf(errInvalidArchive, "invalid whiteout name: %v", base)
232+
}
233+
223234
if err := os.RemoveAll(originalPath); err != nil {
224235
return 0, err
225236
}

‎archive/tar_test.go‎

Lines changed: 166 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,12 +217,47 @@ func TestBreakouts(t *testing.T) {
217217
return nil
218218
}
219219
}
220+
fileValue := func(f1 string, content []byte) func(string) error {
221+
return func(root string) error {
222+
b, err := ioutil.ReadFile(filepath.Join(root, f1))
223+
if err != nil {
224+
return err
225+
}
226+
if bytes.Compare(b, content) != 0 {
227+
return errors.Errorf("content differs: expected %v, got %v", content, b)
228+
}
229+
return nil
230+
}
231+
}
232+
fileNotExists := func(f1 string) func(string) error {
233+
return func(root string) error {
234+
_, err := os.Lstat(filepath.Join(root, f1))
235+
if err == nil {
236+
return errors.New("file exists")
237+
} else if !os.IsNotExist(err) {
238+
return err
239+
}
240+
return nil
241+
}
242+
243+
}
244+
all := func(funcs ...func(string) error) func(string) error {
245+
return func(root string) error {
246+
for _, f := range funcs {
247+
if err := f(root); err != nil {
248+
return err
249+
}
250+
}
251+
return nil
252+
}
253+
}
220254

221255
breakouts := []struct {
222256
name string
223257
w WriterToTar
224258
apply fstest.Applier
225259
validator func(string) error
260+
err error
226261
}{
227262
{
228263
name: "SymlinkAbsolute",
@@ -468,10 +503,129 @@ func TestBreakouts(t *testing.T) {
468503
),
469504
validator: notSameFile("/localetc/localpasswd", "/etc/passwd"),
470505
},
506+
{
507+
name: "WhiteoutRootParent",
508+
apply: fstest.Apply(
509+
fstest.CreateDir("/etc/", 0755),
510+
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
511+
),
512+
w: TarAll(
513+
tc.File(".wh...", []byte{}, 0644), // Should wipe out whole directory
514+
),
515+
err: errInvalidArchive,
516+
},
517+
{
518+
name: "WhiteoutParent",
519+
apply: fstest.Apply(
520+
fstest.CreateDir("/etc/", 0755),
521+
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
522+
),
523+
w: TarAll(
524+
tc.File("etc/.wh...", []byte{}, 0644),
525+
),
526+
err: errInvalidArchive,
527+
},
528+
{
529+
name: "WhiteoutRoot",
530+
apply: fstest.Apply(
531+
fstest.CreateDir("/etc/", 0755),
532+
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
533+
),
534+
w: TarAll(
535+
tc.File(".wh..", []byte{}, 0644),
536+
),
537+
err: errInvalidArchive,
538+
},
539+
{
540+
name: "WhiteoutCurrentDirectory",
541+
apply: fstest.Apply(
542+
fstest.CreateDir("/etc/", 0755),
543+
fstest.CreateFile("/etc/passwd", []byte("inside"), 0644),
544+
),
545+
w: TarAll(
546+
tc.File("etc/.wh..", []byte{}, 0644), // Should wipe out whole directory
547+
),
548+
err: errInvalidArchive,
549+
},
550+
{
551+
name: "WhiteoutSymlink",
552+
apply: fstest.Apply(
553+
fstest.CreateDir("/etc/", 0755),
554+
fstest.CreateFile("/etc/passwd", []byte("all users"), 0644),
555+
fstest.Symlink("/etc", "localetc"),
556+
),
557+
w: TarAll(
558+
tc.File(".wh.localetc", []byte{}, 0644), // Should wipe out whole directory
559+
),
560+
validator: all(
561+
fileValue("etc/passwd", []byte("all users")),
562+
fileNotExists("localetc"),
563+
),
564+
},
565+
{
566+
// TODO: This test should change once archive apply is disallowing
567+
// symlinks as parents in the name
568+
name: "WhiteoutSymlinkPath",
569+
apply: fstest.Apply(
570+
fstest.CreateDir("/etc/", 0755),
571+
fstest.CreateFile("/etc/passwd", []byte("all users"), 0644),
572+
fstest.CreateFile("/etc/whitedout", []byte("ahhhh whiteout"), 0644),
573+
fstest.Symlink("/etc", "localetc"),
574+
),
575+
w: TarAll(
576+
tc.File("localetc/.wh.whitedout", []byte{}, 0644),
577+
),
578+
validator: all(
579+
fileValue("etc/passwd", []byte("all users")),
580+
fileNotExists("etc/whitedout"),
581+
),
582+
},
583+
{
584+
name: "WhiteoutDirectoryName",
585+
apply: fstest.Apply(
586+
fstest.CreateDir("/etc/", 0755),
587+
fstest.CreateFile("/etc/passwd", []byte("all users"), 0644),
588+
fstest.CreateFile("/etc/whitedout", []byte("ahhhh whiteout"), 0644),
589+
fstest.Symlink("/etc", "localetc"),
590+
),
591+
w: TarAll(
592+
tc.File(".wh.etc/somefile", []byte("non-empty"), 0644),
593+
),
594+
validator: all(
595+
fileValue("etc/passwd", []byte("all users")),
596+
fileValue(".wh.etc/somefile", []byte("non-empty")),
597+
),
598+
},
599+
{
600+
name: "WhiteoutDeadSymlinkParent",
601+
apply: fstest.Apply(
602+
fstest.CreateDir("/etc/", 0755),
603+
fstest.CreateFile("/etc/passwd", []byte("all users"), 0644),
604+
fstest.Symlink("/dne", "localetc"),
605+
),
606+
w: TarAll(
607+
tc.File("localetc/.wh.etc", []byte{}, 0644),
608+
),
609+
// no-op, remove does not
610+
validator: fileValue("etc/passwd", []byte("all users")),
611+
},
612+
{
613+
name: "WhiteoutRelativePath",
614+
apply: fstest.Apply(
615+
fstest.CreateDir("/etc/", 0755),
616+
fstest.CreateFile("/etc/passwd", []byte("all users"), 0644),
617+
fstest.Symlink("/dne", "localetc"),
618+
),
619+
w: TarAll(
620+
tc.File("dne/../.wh.etc", []byte{}, 0644),
621+
),
622+
// resolution ends up just removing etc
623+
validator: fileNotExists("etc/passwd"),
624+
},
471625
}
472626

473627
for _, bo := range breakouts {
474-
t.Run(bo.name, makeWriterToTarTest(bo.w, bo.apply, bo.validator))
628+
t.Run(bo.name, makeWriterToTarTest(bo.w, bo.apply, bo.validator, bo.err))
475629
}
476630
}
477631

@@ -501,6 +655,7 @@ func TestApplyTar(t *testing.T) {
501655
w WriterToTar
502656
apply fstest.Applier
503657
validator func(string) error
658+
err error
504659
}{
505660
{
506661
name: "DirectoryCreation",
@@ -525,7 +680,7 @@ func TestApplyTar(t *testing.T) {
525680
}
526681

527682
for _, at := range tests {
528-
t.Run(at.name, makeWriterToTarTest(at.w, at.apply, at.validator))
683+
t.Run(at.name, makeWriterToTarTest(at.w, at.apply, at.validator, at.err))
529684
}
530685
}
531686

@@ -636,7 +791,7 @@ func testDiffApply(appliers ...fstest.Applier) error {
636791
return fstest.CheckDirectoryEqual(td, dest)
637792
}
638793

639-
func makeWriterToTarTest(wt WriterToTar, a fstest.Applier, validate func(string) error) func(*testing.T) {
794+
func makeWriterToTarTest(wt WriterToTar, a fstest.Applier, validate func(string) error, applyErr error) func(*testing.T) {
640795
return func(t *testing.T) {
641796
td, err := ioutil.TempDir("", "test-writer-to-tar-")
642797
if err != nil {
@@ -653,7 +808,14 @@ func makeWriterToTarTest(wt WriterToTar, a fstest.Applier, validate func(string)
653808
tr := TarFromWriterTo(wt)
654809

655810
if _, err := Apply(context.Background(), td, tr); err != nil {
656-
t.Fatalf("Failed to apply tar: %v", err)
811+
if applyErr == nil {
812+
t.Fatalf("Failed to apply tar: %v", err)
813+
} else if errors.Cause(err) != applyErr {
814+
t.Fatalf("Unexpected apply error: %v, expected %v", err, applyErr)
815+
}
816+
return
817+
} else if applyErr != nil {
818+
t.Fatalf("Expected apply error, got none: %v", applyErr)
657819
}
658820

659821
if validate != nil {

0 commit comments

Comments
 (0)