Skip to content

Commit e134c03

Browse files
ctalledothaJeztah
authored andcommitted
archive: add regression tests for tar path-traversal containment
Both tests fail against the previous (vulnerable) code and pass with the hardening in this branch: - TestUntarParentTraversalContained: entries named ".." / "../pwned" are rejected and never write into the destination's parent. - TestUntarSiblingPrefixContained: a hardlink whose target resolves to a sibling sharing the destination's path prefix (dest "base/dest", sibling "base/dest-evil") is rejected, rather than accepted by a string HasPrefix check. Windows-specific coverage (":" / "\" handling) needs Windows CI and remains a follow-up. Signed-off-by: Cesar Talledo <cesar.talledo@docker.com>
1 parent 2f43d86 commit e134c03

1 file changed

Lines changed: 65 additions & 0 deletions

File tree

archive_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,71 @@ func TestUntarInvalidFilenames(t *testing.T) {
717717
}
718718
}
719719

720+
// TestUntarParentTraversalContained verifies that entries whose names traverse
721+
// above the destination (including a bare "..") are rejected and never write
722+
// into the destination's parent. Regression test for the "write to the parent
723+
// of the extraction root" breakout.
724+
func TestUntarParentTraversalContained(t *testing.T) {
725+
base := t.TempDir()
726+
dest := filepath.Join(base, "dest")
727+
assert.NilError(t, os.Mkdir(dest, 0o755))
728+
729+
for _, name := range []string{"..", "../pwned", "../../pwned"} {
730+
buf := &bytes.Buffer{}
731+
tw := tar.NewWriter(buf)
732+
assert.NilError(t, tw.WriteHeader(&tar.Header{
733+
Name: name, Typeflag: tar.TypeReg, Mode: 0o644, Size: int64(len("bad")),
734+
}))
735+
_, err := tw.Write([]byte("bad"))
736+
assert.NilError(t, err)
737+
assert.NilError(t, tw.Close())
738+
739+
// If Untar returns an error it must be a breakoutError; regardless,
740+
// nothing may be written above dest.
741+
if err := Untar(buf, dest, nil); err != nil {
742+
assert.Assert(t, errors.As(err, new(breakoutError)), "name=%q: got %v", name, err)
743+
}
744+
}
745+
746+
// dest's parent must still contain only "dest" -- nothing escaped.
747+
entries, err := os.ReadDir(base)
748+
assert.NilError(t, err)
749+
assert.Equal(t, len(entries), 1, "unexpected escape into parent: %v", entries)
750+
assert.Equal(t, entries[0].Name(), "dest")
751+
}
752+
753+
// TestUntarSiblingPrefixContained verifies that a symlink whose target is a
754+
// sibling directory sharing the destination's path prefix (dest "base/dest",
755+
// sibling "base/dest-evil") cannot be written through. Regression test for the
756+
// old string-prefix (HasPrefix) containment check, which treated such a sibling
757+
// as inside the destination.
758+
func TestUntarSiblingPrefixContained(t *testing.T) {
759+
base := t.TempDir()
760+
dest := filepath.Join(base, "dest")
761+
assert.NilError(t, os.Mkdir(dest, 0o755))
762+
// Prefix-sharing sibling with a sentinel file.
763+
evil := filepath.Join(base, "dest-evil")
764+
assert.NilError(t, os.Mkdir(evil, 0o755))
765+
766+
assert.NilError(t, os.WriteFile(filepath.Join(evil, "secret"), []byte("secret"), 0o600))
767+
768+
buf := &bytes.Buffer{}
769+
tw := tar.NewWriter(buf)
770+
// A hardlink whose target resolves to the prefix-sharing sibling. The old
771+
// strings.HasPrefix check accepted this because "base/dest-evil/secret"
772+
// starts with "base/dest".
773+
assert.NilError(t, tw.WriteHeader(&tar.Header{
774+
Name: "grab", Typeflag: tar.TypeLink, Linkname: "../dest-evil/secret", Mode: 0o644,
775+
}))
776+
assert.NilError(t, tw.Close())
777+
778+
_ = Untar(buf, dest, nil) // may error; we only require containment
779+
780+
// No hardlink to the sibling's secret may be created inside dest.
781+
_, statErr := os.Stat(filepath.Join(dest, "grab"))
782+
assert.Assert(t, os.IsNotExist(statErr), "hardlink to prefix-sibling created: %v", statErr)
783+
}
784+
720785
func TestUntarHardlinkToSymlink(t *testing.T) {
721786
skip.If(t, runtime.GOOS != "windows" && os.Getuid() != 0, "skipping test that requires root")
722787
for i, headers := range [][]*tar.Header{

0 commit comments

Comments
 (0)