Skip to content

Commit 8568349

Browse files
committed
Harden tar extraction against zip-slip path traversal
Reject absolute paths in tar entry names and validate symlink targets in the tar-entry namespace to prevent directory escape. Validation is routing-agnostic so it works correctly with multi-destination extraction (caches/ → GRADLE_USER_HOME, configuration-cache/ → project/.gradle/).
1 parent 2da97e1 commit 8568349

3 files changed

Lines changed: 166 additions & 2 deletions

File tree

cmd/gradle-cache/extract_darwin.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ readLoop:
107107

108108
// Reject path traversal before passing to targetFn.
109109
name := filepath.Clean(hdr.Name)
110-
if strings.HasPrefix(name, "..") {
110+
if strings.HasPrefix(name, "..") || filepath.IsAbs(name) {
111111
readErr = errors.Errorf("tar entry %q escapes destination directory", hdr.Name)
112112
break
113113
}
@@ -150,6 +150,18 @@ readLoop:
150150
continue
151151
}
152152
}
153+
// Validate symlink target does not escape the archive root.
154+
// Resolve in tar-entry namespace (before routing) so validation
155+
// is independent of which destination directory entries are routed to.
156+
if filepath.IsAbs(hdr.Linkname) {
157+
readErr = errors.Errorf("symlink %q -> %q: absolute symlink target not allowed", hdr.Name, hdr.Linkname)
158+
break readLoop
159+
}
160+
resolvedLink := filepath.Clean(filepath.Join(filepath.Dir(name), hdr.Linkname))
161+
if strings.HasPrefix(resolvedLink, "..") {
162+
readErr = errors.Errorf("symlink %q -> %q escapes destination directory", hdr.Name, hdr.Linkname)
163+
break readLoop
164+
}
153165
if err := ensureDir(filepath.Dir(target)); err != nil {
154166
readErr = errors.Errorf("mkdir for symlink %s: %w", hdr.Name, err)
155167
break readLoop

cmd/gradle-cache/extract_default.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ loop:
119119
}
120120

121121
name := filepath.Clean(hdr.Name)
122-
if strings.HasPrefix(name, "..") {
122+
if strings.HasPrefix(name, "..") || filepath.IsAbs(name) {
123123
readErr = errors.Errorf("tar entry %q escapes destination directory", hdr.Name)
124124
break
125125
}
@@ -185,6 +185,18 @@ loop:
185185
continue
186186
}
187187
}
188+
// Validate symlink target does not escape the archive root.
189+
// Resolve in tar-entry namespace (before routing) so validation
190+
// is independent of which destination directory entries are routed to.
191+
if filepath.IsAbs(hdr.Linkname) {
192+
readErr = errors.Errorf("symlink %q -> %q: absolute symlink target not allowed", hdr.Name, hdr.Linkname)
193+
break loop
194+
}
195+
resolvedLink := filepath.Clean(filepath.Join(filepath.Dir(name), hdr.Linkname))
196+
if strings.HasPrefix(resolvedLink, "..") {
197+
readErr = errors.Errorf("symlink %q -> %q escapes destination directory", hdr.Name, hdr.Linkname)
198+
break loop
199+
}
188200
if err := ensureDir(filepath.Dir(target)); err != nil {
189201
readErr = errors.Errorf("mkdir for symlink %s: %w", hdr.Name, err)
190202
break loop

cmd/gradle-cache/main_test.go

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,146 @@ func BenchmarkDeltaScanReal(b *testing.B) {
834834
b.ReportMetric(float64(totalFiles), "files/op")
835835
}
836836

837+
// ─── Zip-slip / path-traversal tests ──────────────────────────────────────────
838+
839+
// TestExtractRejectsPathTraversal verifies that tar entries with ".." in the
840+
// name or symlinks pointing outside the destination are rejected.
841+
func TestExtractRejectsPathTraversal(t *testing.T) {
842+
// Helper: build a tar archive in memory from a list of entries.
843+
type entry struct {
844+
name string
845+
typeflag byte
846+
linkname string
847+
body string
848+
}
849+
buildTar := func(entries []entry) *bytes.Buffer {
850+
var buf bytes.Buffer
851+
tw := archive_tar.NewWriter(&buf)
852+
for _, e := range entries {
853+
hdr := &archive_tar.Header{
854+
Name: e.name,
855+
Typeflag: e.typeflag,
856+
Mode: 0o644,
857+
Size: int64(len(e.body)),
858+
Linkname: e.linkname,
859+
}
860+
if e.typeflag == archive_tar.TypeDir {
861+
hdr.Mode = 0o755
862+
hdr.Size = 0
863+
}
864+
must(t, tw.WriteHeader(hdr))
865+
if len(e.body) > 0 {
866+
_, err := tw.Write([]byte(e.body))
867+
must(t, err)
868+
}
869+
}
870+
must(t, tw.Close())
871+
return &buf
872+
}
873+
874+
for _, tc := range []struct {
875+
name string
876+
entries []entry
877+
}{
878+
{
879+
name: "dotdot_file",
880+
entries: []entry{
881+
{name: "../etc/passwd", typeflag: archive_tar.TypeReg, body: "pwned"},
882+
},
883+
},
884+
{
885+
name: "dotdot_nested_file",
886+
entries: []entry{
887+
{name: "foo/../../etc/passwd", typeflag: archive_tar.TypeReg, body: "pwned"},
888+
},
889+
},
890+
{
891+
name: "absolute_file",
892+
entries: []entry{
893+
{name: "/etc/passwd", typeflag: archive_tar.TypeReg, body: "pwned"},
894+
},
895+
},
896+
{
897+
name: "symlink_absolute_escape",
898+
entries: []entry{
899+
{name: "link", typeflag: archive_tar.TypeSymlink, linkname: "/etc/passwd"},
900+
},
901+
},
902+
{
903+
name: "symlink_relative_escape",
904+
entries: []entry{
905+
{name: "link", typeflag: archive_tar.TypeSymlink, linkname: "../../etc/passwd"},
906+
},
907+
},
908+
} {
909+
t.Run(tc.name, func(t *testing.T) {
910+
dstDir := t.TempDir()
911+
buf := buildTar(tc.entries)
912+
err := extractTarPlatform(bytes.NewReader(buf.Bytes()), dstDir)
913+
if err == nil {
914+
t.Fatal("expected error for path-traversal entry, got nil")
915+
}
916+
if !strings.Contains(err.Error(), "escapes") && !strings.Contains(err.Error(), "not allowed") {
917+
t.Fatalf("expected 'escapes' or 'not allowed' in error, got: %v", err)
918+
}
919+
})
920+
}
921+
}
922+
923+
// TestExtractRoutedSymlinkWithinArchive verifies that symlinks between routed
924+
// directories (e.g. configuration-cache/ → caches/) are accepted when they
925+
// stay within the archive root, even though the two directories are extracted
926+
// to different filesystem locations.
927+
func TestExtractRoutedSymlinkWithinArchive(t *testing.T) {
928+
var buf bytes.Buffer
929+
tw := archive_tar.NewWriter(&buf)
930+
931+
// A file in caches/ and a symlink in configuration-cache/ pointing to it.
932+
for _, e := range []struct {
933+
name, linkname, body string
934+
typeflag byte
935+
}{
936+
{name: "caches/modules/foo.bin", body: "data", typeflag: archive_tar.TypeReg},
937+
{name: "configuration-cache/link", linkname: "../caches/modules/foo.bin", typeflag: archive_tar.TypeSymlink},
938+
} {
939+
hdr := &archive_tar.Header{
940+
Name: e.name,
941+
Typeflag: e.typeflag,
942+
Mode: 0o644,
943+
Size: int64(len(e.body)),
944+
Linkname: e.linkname,
945+
}
946+
must(t, tw.WriteHeader(hdr))
947+
if len(e.body) > 0 {
948+
_, err := tw.Write([]byte(e.body))
949+
must(t, err)
950+
}
951+
}
952+
must(t, tw.Close())
953+
954+
gradleHome := t.TempDir()
955+
projectDir := t.TempDir()
956+
dotGradle := filepath.Join(projectDir, ".gradle")
957+
958+
rules := []extractRule{
959+
{prefix: "caches/", baseDir: gradleHome},
960+
{prefix: "configuration-cache/", baseDir: dotGradle},
961+
}
962+
targetFn := func(name string) string {
963+
for _, rule := range rules {
964+
if strings.HasPrefix(name, rule.prefix) {
965+
return filepath.Join(rule.baseDir, name)
966+
}
967+
}
968+
return filepath.Join(projectDir, name)
969+
}
970+
971+
err := extractTarPlatformRouted(bytes.NewReader(buf.Bytes()), targetFn, false)
972+
if err != nil {
973+
t.Fatalf("expected routed cross-directory symlink to succeed, got: %v", err)
974+
}
975+
}
976+
837977
// ─── Extraction benchmark ─────────────────────────────────────────────────────
838978

839979
// BenchmarkExtract measures extractTarPlatformRouted throughput against a

0 commit comments

Comments
 (0)