1. docker
  2. Commit 221617dbcd9431f14a3779d8bac9aba52f78ea21
Browse code

archive: add breakout tests

Signed-off-by: Tibor Vass <teabee89@gmail.com>

Conflicts:
pkg/archive/archive.go
fixed conflict which git couldn't fix with the added BreakoutError

Conflicts:
pkg/archive/archive_test.go
fixed conflict in imports

Tibor Vass authored on 2014/10/21 04:35:48
Showing 4 changed files
pkg/archive/archive.go
History View file @ 221617d
... ... 
@@ -42,6 +42,11 @@ type (
42 42 
 	Archiver struct {
43 43 
 		Untar func(io.Reader, string, *TarOptions) error
44 44 
 	}
45 
+
46 
+	// breakoutError is used to differentiate errors related to breaking out
47 
+	// When testing archive breakout in the unit tests, this error is expected
48 
+	// in order for the test to pass.
49 
+	breakoutError error
45 50 
 )
46 51
47 52 
 var (
pkg/archive/archive_test.go
History View file @ 221617d
... ... 
@@ -8,6 +8,7 @@ import (
8 8 
 	"os"
9 9 
 	"os/exec"
10 10 
 	"path"
11 
+	"path/filepath"
11 12 
 	"syscall"
12 13 
 	"testing"
13 14 
 	"time"
... ... 
@@ -214,7 +215,12 @@ func TestTarWithOptions(t *testing.T) {
214 214 
 // Failing prevents the archives from being uncompressed during ADD
215 215 
 func TestTypeXGlobalHeaderDoesNotFail(t *testing.T) {
216 216 
 	hdr := tar.Header{Typeflag: tar.TypeXGlobalHeader}
217 
-	err := createTarFile("pax_global_header", "some_dir", &hdr, nil, true)
217 
+	tmpDir, err := ioutil.TempDir("", "docker-test-archive-pax-test")
218 
+	if err != nil {
219 
+		t.Fatal(err)
220 
+	}
221 
+	defer os.RemoveAll(tmpDir)
222 
+	err = createTarFile(filepath.Join(tmpDir, "pax_global_header"), tmpDir, &hdr, nil, true)
218 223 
 	if err != nil {
219 224 
 		t.Fatal(err)
220 225 
 	}
... ... 
@@ -403,3 +409,187 @@ func BenchmarkTarUntarWithLinks(b *testing.B) {
403 403 
 		os.RemoveAll(target)
404 404 
 	}
405 405 
 }
406 
+
407 
+func TestUntarInvalidFilenames(t *testing.T) {
408 
+	for i, headers := range [][]*tar.Header{
409 
+		{
410 
+			{
411 
+				Name:     "../victim/dotdot",
412 
+				Typeflag: tar.TypeReg,
413 
+				Mode:     0644,
414 
+			},
415 
+		},
416 
+		{
417 
+			{
418 
+				// Note the leading slash
419 
+				Name:     "/../victim/slash-dotdot",
420 
+				Typeflag: tar.TypeReg,
421 
+				Mode:     0644,
422 
+			},
423 
+		},
424 
+	} {
425 
+		if err := testBreakout("untar", "docker-TestUntarInvalidFilenames", headers); err != nil {
426 
+			t.Fatalf("i=%d. %v", i, err)
427 
+		}
428 
+	}
429 
+}
430 
+
431 
+func TestUntarInvalidHardlink(t *testing.T) {
432 
+	for i, headers := range [][]*tar.Header{
433 
+		{ // try reading victim/hello (../)
434 
+			{
435 
+				Name:     "dotdot",
436 
+				Typeflag: tar.TypeLink,
437 
+				Linkname: "../victim/hello",
438 
+				Mode:     0644,
439 
+			},
440 
+		},
441 
+		{ // try reading victim/hello (/../)
442 
+			{
443 
+				Name:     "slash-dotdot",
444 
+				Typeflag: tar.TypeLink,
445 
+				// Note the leading slash
446 
+				Linkname: "/../victim/hello",
447 
+				Mode:     0644,
448 
+			},
449 
+		},
450 
+		{ // try writing victim/file
451 
+			{
452 
+				Name:     "loophole-victim",
453 
+				Typeflag: tar.TypeLink,
454 
+				Linkname: "../victim",
455 
+				Mode:     0755,
456 
+			},
457 
+			{
458 
+				Name:     "loophole-victim/file",
459 
+				Typeflag: tar.TypeReg,
460 
+				Mode:     0644,
461 
+			},
462 
+		},
463 
+		{ // try reading victim/hello (hardlink, symlink)
464 
+			{
465 
+				Name:     "loophole-victim",
466 
+				Typeflag: tar.TypeLink,
467 
+				Linkname: "../victim",
468 
+				Mode:     0755,
469 
+			},
470 
+			{
471 
+				Name:     "symlink",
472 
+				Typeflag: tar.TypeSymlink,
473 
+				Linkname: "loophole-victim/hello",
474 
+				Mode:     0644,
475 
+			},
476 
+		},
477 
+		{ // Try reading victim/hello (hardlink, hardlink)
478 
+			{
479 
+				Name:     "loophole-victim",
480 
+				Typeflag: tar.TypeLink,
481 
+				Linkname: "../victim",
482 
+				Mode:     0755,
483 
+			},
484 
+			{
485 
+				Name:     "hardlink",
486 
+				Typeflag: tar.TypeLink,
487 
+				Linkname: "loophole-victim/hello",
488 
+				Mode:     0644,
489 
+			},
490 
+		},
491 
+		{ // Try removing victim directory (hardlink)
492 
+			{
493 
+				Name:     "loophole-victim",
494 
+				Typeflag: tar.TypeLink,
495 
+				Linkname: "../victim",
496 
+				Mode:     0755,
497 
+			},
498 
+			{
499 
+				Name:     "loophole-victim",
500 
+				Typeflag: tar.TypeReg,
501 
+				Mode:     0644,
502 
+			},
503 
+		},
504 
+	} {
505 
+		if err := testBreakout("untar", "docker-TestUntarInvalidHardlink", headers); err != nil {
506 
+			t.Fatalf("i=%d. %v", i, err)
507 
+		}
508 
+	}
509 
+}
510 
+
511 
+func TestUntarInvalidSymlink(t *testing.T) {
512 
+	for i, headers := range [][]*tar.Header{
513 
+		{ // try reading victim/hello (../)
514 
+			{
515 
+				Name:     "dotdot",
516 
+				Typeflag: tar.TypeSymlink,
517 
+				Linkname: "../victim/hello",
518 
+				Mode:     0644,
519 
+			},
520 
+		},
521 
+		{ // try reading victim/hello (/../)
522 
+			{
523 
+				Name:     "slash-dotdot",
524 
+				Typeflag: tar.TypeSymlink,
525 
+				// Note the leading slash
526 
+				Linkname: "/../victim/hello",
527 
+				Mode:     0644,
528 
+			},
529 
+		},
530 
+		{ // try writing victim/file
531 
+			{
532 
+				Name:     "loophole-victim",
533 
+				Typeflag: tar.TypeSymlink,
534 
+				Linkname: "../victim",
535 
+				Mode:     0755,
536 
+			},
537 
+			{
538 
+				Name:     "loophole-victim/file",
539 
+				Typeflag: tar.TypeReg,
540 
+				Mode:     0644,
541 
+			},
542 
+		},
543 
+		{ // try reading victim/hello (symlink, symlink)
544 
+			{
545 
+				Name:     "loophole-victim",
546 
+				Typeflag: tar.TypeSymlink,
547 
+				Linkname: "../victim",
548 
+				Mode:     0755,
549 
+			},
550 
+			{
551 
+				Name:     "symlink",
552 
+				Typeflag: tar.TypeSymlink,
553 
+				Linkname: "loophole-victim/hello",
554 
+				Mode:     0644,
555 
+			},
556 
+		},
557 
+		{ // try reading victim/hello (symlink, hardlink)
558 
+			{
559 
+				Name:     "loophole-victim",
560 
+				Typeflag: tar.TypeSymlink,
561 
+				Linkname: "../victim",
562 
+				Mode:     0755,
563 
+			},
564 
+			{
565 
+				Name:     "hardlink",
566 
+				Typeflag: tar.TypeLink,
567 
+				Linkname: "loophole-victim/hello",
568 
+				Mode:     0644,
569 
+			},
570 
+		},
571 
+		{ // try removing victim directory (symlink)
572 
+			{
573 
+				Name:     "loophole-victim",
574 
+				Typeflag: tar.TypeSymlink,
575 
+				Linkname: "../victim",
576 
+				Mode:     0755,
577 
+			},
578 
+			{
579 
+				Name:     "loophole-victim",
580 
+				Typeflag: tar.TypeReg,
581 
+				Mode:     0644,
582 
+			},
583 
+		},
584 
+	} {
585 
+		if err := testBreakout("untar", "docker-TestUntarInvalidSymlink", headers); err != nil {
586 
+			t.Fatalf("i=%d. %v", i, err)
587 
+		}
588 
+	}
589 
+}
pkg/archive/diff_test.go
History View file @ 221617d
406 590 
new file mode 100644
... ... 
@@ -0,0 +1,191 @@
0 
+package archive
1 
+
2 
+import (
3 
+	"testing"
4 
+
5 
+	"github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
6 
+)
7 
+
8 
+func TestApplyLayerInvalidFilenames(t *testing.T) {
9 
+	for i, headers := range [][]*tar.Header{
10 
+		{
11 
+			{
12 
+				Name:     "../victim/dotdot",
13 
+				Typeflag: tar.TypeReg,
14 
+				Mode:     0644,
15 
+			},
16 
+		},
17 
+		{
18 
+			{
19 
+				// Note the leading slash
20 
+				Name:     "/../victim/slash-dotdot",
21 
+				Typeflag: tar.TypeReg,
22 
+				Mode:     0644,
23 
+			},
24 
+		},
25 
+	} {
26 
+		if err := testBreakout("applylayer", "docker-TestApplyLayerInvalidFilenames", headers); err != nil {
27 
+			t.Fatalf("i=%d. %v", i, err)
28 
+		}
29 
+	}
30 
+}
31 
+
32 
+func TestApplyLayerInvalidHardlink(t *testing.T) {
33 
+	for i, headers := range [][]*tar.Header{
34 
+		{ // try reading victim/hello (../)
35 
+			{
36 
+				Name:     "dotdot",
37 
+				Typeflag: tar.TypeLink,
38 
+				Linkname: "../victim/hello",
39 
+				Mode:     0644,
40 
+			},
41 
+		},
42 
+		{ // try reading victim/hello (/../)
43 
+			{
44 
+				Name:     "slash-dotdot",
45 
+				Typeflag: tar.TypeLink,
46 
+				// Note the leading slash
47 
+				Linkname: "/../victim/hello",
48 
+				Mode:     0644,
49 
+			},
50 
+		},
51 
+		{ // try writing victim/file
52 
+			{
53 
+				Name:     "loophole-victim",
54 
+				Typeflag: tar.TypeLink,
55 
+				Linkname: "../victim",
56 
+				Mode:     0755,
57 
+			},
58 
+			{
59 
+				Name:     "loophole-victim/file",
60 
+				Typeflag: tar.TypeReg,
61 
+				Mode:     0644,
62 
+			},
63 
+		},
64 
+		{ // try reading victim/hello (hardlink, symlink)
65 
+			{
66 
+				Name:     "loophole-victim",
67 
+				Typeflag: tar.TypeLink,
68 
+				Linkname: "../victim",
69 
+				Mode:     0755,
70 
+			},
71 
+			{
72 
+				Name:     "symlink",
73 
+				Typeflag: tar.TypeSymlink,
74 
+				Linkname: "loophole-victim/hello",
75 
+				Mode:     0644,
76 
+			},
77 
+		},
78 
+		{ // Try reading victim/hello (hardlink, hardlink)
79 
+			{
80 
+				Name:     "loophole-victim",
81 
+				Typeflag: tar.TypeLink,
82 
+				Linkname: "../victim",
83 
+				Mode:     0755,
84 
+			},
85 
+			{
86 
+				Name:     "hardlink",
87 
+				Typeflag: tar.TypeLink,
88 
+				Linkname: "loophole-victim/hello",
89 
+				Mode:     0644,
90 
+			},
91 
+		},
92 
+		{ // Try removing victim directory (hardlink)
93 
+			{
94 
+				Name:     "loophole-victim",
95 
+				Typeflag: tar.TypeLink,
96 
+				Linkname: "../victim",
97 
+				Mode:     0755,
98 
+			},
99 
+			{
100 
+				Name:     "loophole-victim",
101 
+				Typeflag: tar.TypeReg,
102 
+				Mode:     0644,
103 
+			},
104 
+		},
105 
+	} {
106 
+		if err := testBreakout("applylayer", "docker-TestApplyLayerInvalidHardlink", headers); err != nil {
107 
+			t.Fatalf("i=%d. %v", i, err)
108 
+		}
109 
+	}
110 
+}
111 
+
112 
+func TestApplyLayerInvalidSymlink(t *testing.T) {
113 
+	for i, headers := range [][]*tar.Header{
114 
+		{ // try reading victim/hello (../)
115 
+			{
116 
+				Name:     "dotdot",
117 
+				Typeflag: tar.TypeSymlink,
118 
+				Linkname: "../victim/hello",
119 
+				Mode:     0644,
120 
+			},
121 
+		},
122 
+		{ // try reading victim/hello (/../)
123 
+			{
124 
+				Name:     "slash-dotdot",
125 
+				Typeflag: tar.TypeSymlink,
126 
+				// Note the leading slash
127 
+				Linkname: "/../victim/hello",
128 
+				Mode:     0644,
129 
+			},
130 
+		},
131 
+		{ // try writing victim/file
132 
+			{
133 
+				Name:     "loophole-victim",
134 
+				Typeflag: tar.TypeSymlink,
135 
+				Linkname: "../victim",
136 
+				Mode:     0755,
137 
+			},
138 
+			{
139 
+				Name:     "loophole-victim/file",
140 
+				Typeflag: tar.TypeReg,
141 
+				Mode:     0644,
142 
+			},
143 
+		},
144 
+		{ // try reading victim/hello (symlink, symlink)
145 
+			{
146 
+				Name:     "loophole-victim",
147 
+				Typeflag: tar.TypeSymlink,
148 
+				Linkname: "../victim",
149 
+				Mode:     0755,
150 
+			},
151 
+			{
152 
+				Name:     "symlink",
153 
+				Typeflag: tar.TypeSymlink,
154 
+				Linkname: "loophole-victim/hello",
155 
+				Mode:     0644,
156 
+			},
157 
+		},
158 
+		{ // try reading victim/hello (symlink, hardlink)
159 
+			{
160 
+				Name:     "loophole-victim",
161 
+				Typeflag: tar.TypeSymlink,
162 
+				Linkname: "../victim",
163 
+				Mode:     0755,
164 
+			},
165 
+			{
166 
+				Name:     "hardlink",
167 
+				Typeflag: tar.TypeLink,
168 
+				Linkname: "loophole-victim/hello",
169 
+				Mode:     0644,
170 
+			},
171 
+		},
172 
+		{ // try removing victim directory (symlink)
173 
+			{
174 
+				Name:     "loophole-victim",
175 
+				Typeflag: tar.TypeSymlink,
176 
+				Linkname: "../victim",
177 
+				Mode:     0755,
178 
+			},
179 
+			{
180 
+				Name:     "loophole-victim",
181 
+				Typeflag: tar.TypeReg,
182 
+				Mode:     0644,
183 
+			},
184 
+		},
185 
+	} {
186 
+		if err := testBreakout("applylayer", "docker-TestApplyLayerInvalidSymlink", headers); err != nil {
187 
+			t.Fatalf("i=%d. %v", i, err)
188 
+		}
189 
+	}
190 
+}
pkg/archive/utils_test.go
History View file @ 221617d
0 191 
new file mode 100644
... ... 
@@ -0,0 +1,166 @@
0 
+package archive
1 
+
2 
+import (
3 
+	"bytes"
4 
+	"fmt"
5 
+	"io"
6 
+	"io/ioutil"
7 
+	"os"
8 
+	"path/filepath"
9 
+	"time"
10 
+
11 
+	"github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
12 
+)
13 
+
14 
+var testUntarFns = map[string]func(string, io.Reader) error{
15 
+	"untar": func(dest string, r io.Reader) error {
16 
+		return Untar(r, dest, nil)
17 
+	},
18 
+	"applylayer": func(dest string, r io.Reader) error {
19 
+		return ApplyLayer(dest, ArchiveReader(r))
20 
+	},
21 
+}
22 
+
23 
+// testBreakout is a helper function that, within the provided `tmpdir` directory,
24 
+// creates a `victim` folder with a generated `hello` file in it.
25 
+// `untar` extracts to a directory named `dest`, the tar file created from `headers`.
26 
+//
27 
+// Here are the tested scenarios:
28 
+// - removed `victim` folder				(write)
29 
+// - removed files from `victim` folder			(write)
30 
+// - new files in `victim` folder			(write)
31 
+// - modified files in `victim` folder			(write)
32 
+// - file in `dest` with same content as `victim/hello` (read)
33 
+//
34 
+// When using testBreakout make sure you cover one of the scenarios listed above.
35 
+func testBreakout(untarFn string, tmpdir string, headers []*tar.Header) error {
36 
+	tmpdir, err := ioutil.TempDir("", tmpdir)
37 
+	if err != nil {
38 
+		return err
39 
+	}
40 
+	defer os.RemoveAll(tmpdir)
41 
+
42 
+	dest := filepath.Join(tmpdir, "dest")
43 
+	if err := os.Mkdir(dest, 0755); err != nil {
44 
+		return err
45 
+	}
46 
+
47 
+	victim := filepath.Join(tmpdir, "victim")
48 
+	if err := os.Mkdir(victim, 0755); err != nil {
49 
+		return err
50 
+	}
51 
+	hello := filepath.Join(victim, "hello")
52 
+	helloData, err := time.Now().MarshalText()
53 
+	if err != nil {
54 
+		return err
55 
+	}
56 
+	if err := ioutil.WriteFile(hello, helloData, 0644); err != nil {
57 
+		return err
58 
+	}
59 
+	helloStat, err := os.Stat(hello)
60 
+	if err != nil {
61 
+		return err
62 
+	}
63 
+
64 
+	reader, writer := io.Pipe()
65 
+	go func() {
66 
+		t := tar.NewWriter(writer)
67 
+		for _, hdr := range headers {
68 
+			t.WriteHeader(hdr)
69 
+		}
70 
+		t.Close()
71 
+	}()
72 
+
73 
+	untar := testUntarFns[untarFn]
74 
+	if untar == nil {
75 
+		return fmt.Errorf("could not find untar function %q in testUntarFns", untarFn)
76 
+	}
77 
+	if err := untar(dest, reader); err != nil {
78 
+		if _, ok := err.(breakoutError); !ok {
79 
+			// If untar returns an error unrelated to an archive breakout,
80 
+			// then consider this an unexpected error and abort.
81 
+			return err
82 
+		}
83 
+		// Here, untar detected the breakout.
84 
+		// Let's move on verifying that indeed there was no breakout.
85 
+		fmt.Printf("breakoutError: %v\n", err)
86 
+	}
87 
+
88 
+	// Check victim folder
89 
+	f, err := os.Open(victim)
90 
+	if err != nil {
91 
+		// codepath taken if victim folder was removed
92 
+		return fmt.Errorf("archive breakout: error reading %q: %v", victim, err)
93 
+	}
94 
+	defer f.Close()
95 
+
96 
+	// Check contents of victim folder
97 
+	//
98 
+	// We are only interested in getting 2 files from the victim folder, because if all is well
99 
+	// we expect only one result, the `hello` file. If there is a second result, it cannot
100 
+	// hold the same name `hello` and we assume that a new file got created in the victim folder.
101 
+	// That is enough to detect an archive breakout.
102 
+	names, err := f.Readdirnames(2)
103 
+	if err != nil {
104 
+		// codepath taken if victim is not a folder
105 
+		return fmt.Errorf("archive breakout: error reading directory content of %q: %v", victim, err)
106 
+	}
107 
+	for _, name := range names {
108 
+		if name != "hello" {
109 
+			// codepath taken if new file was created in victim folder
110 
+			return fmt.Errorf("archive breakout: new file %q", name)
111 
+		}
112 
+	}
113 
+
114 
+	// Check victim/hello
115 
+	f, err = os.Open(hello)
116 
+	if err != nil {
117 
+		// codepath taken if read permissions were removed
118 
+		return fmt.Errorf("archive breakout: could not lstat %q: %v", hello, err)
119 
+	}
120 
+	defer f.Close()
121 
+	b, err := ioutil.ReadAll(f)
122 
+	if err != nil {
123 
+		return err
124 
+	}
125 
+	fi, err := f.Stat()
126 
+	if err != nil {
127 
+		return err
128 
+	}
129 
+	if helloStat.IsDir() != fi.IsDir() ||
130 
+		// TODO: cannot check for fi.ModTime() change
131 
+		helloStat.Mode() != fi.Mode() ||
132 
+		helloStat.Size() != fi.Size() ||
133 
+		!bytes.Equal(helloData, b) {
134 
+		// codepath taken if hello has been modified
135 
+		return fmt.Errorf("archive breakout: file %q has been modified. Contents: expected=%q, got=%q. FileInfo: expected=%#v, got=%#v.", hello, helloData, b, helloStat, fi)
136 
+	}
137 
+
138 
+	// Check that nothing in dest/ has the same content as victim/hello.
139 
+	// Since victim/hello was generated with time.Now(), it is safe to assume
140 
+	// that any file whose content matches exactly victim/hello, managed somehow
141 
+	// to access victim/hello.
142 
+	return filepath.Walk(dest, func(path string, info os.FileInfo, err error) error {
143 
+		if info.IsDir() {
144 
+			if err != nil {
145 
+				// skip directory if error
146 
+				return filepath.SkipDir
147 
+			}
148 
+			// enter directory
149 
+			return nil
150 
+		}
151 
+		if err != nil {
152 
+			// skip file if error
153 
+			return nil
154 
+		}
155 
+		b, err := ioutil.ReadFile(path)
156 
+		if err != nil {
157 
+			// Houston, we have a problem. Aborting (space)walk.
158 
+			return err
159 
+		}
160 
+		if bytes.Equal(helloData, b) {
161 
+			return fmt.Errorf("archive breakout: file %q has been accessed via %q", hello, path)
162 
+		}
163 
+		return nil
164 
+	})
165 
+}