Skip to content

Commit 137aa7c

Browse files
eljojoeljojo
authored
Merge pull request #6 from vnykmshr/fix/path-traversal-tests
test: add real path traversal tests for tar.gz extraction
2 parents 20b73b1 + 6a29c72 commit 137aa7c

2 files changed

Lines changed: 201 additions & 24 deletions

File tree

‎internal/core/core_test.go‎

Lines changed: 98 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package core
22

33
import (
4+
"archive/tar"
45
"bytes"
6+
"compress/gzip"
57
"strings"
68
"testing"
79
)
@@ -257,13 +259,103 @@ func TestShareFilename(t *testing.T) {
257259
}
258260
}
259261

260-
func TestExtractTarGzPathTraversal(t *testing.T) {
261-
// This test would require creating a malicious tar.gz
262-
// For now, we just verify the function exists and handles empty input
263-
_, err := ExtractTarGz([]byte{})
264-
if err == nil {
265-
t.Error("expected error for empty input")
262+
// createTarGz builds a tar.gz archive in memory with arbitrary entry names.
263+
// This allows crafting malicious archives for security testing.
264+
func createTarGz(t *testing.T, entries map[string]string) []byte {
265+
t.Helper()
266+
var buf bytes.Buffer
267+
gzw := gzip.NewWriter(&buf)
268+
tw := tar.NewWriter(gzw)
269+
270+
for name, content := range entries {
271+
if err := tw.WriteHeader(&tar.Header{
272+
Name: name,
273+
Size: int64(len(content)),
274+
Mode: 0644,
275+
Typeflag: tar.TypeReg,
276+
}); err != nil {
277+
t.Fatalf("writing tar header for %q: %v", name, err)
278+
}
279+
if _, err := tw.Write([]byte(content)); err != nil {
280+
t.Fatalf("writing tar content for %q: %v", name, err)
281+
}
282+
}
283+
284+
// Close tar then gzip explicitly (not defer) to ensure full flush.
285+
if err := tw.Close(); err != nil {
286+
t.Fatalf("closing tar writer: %v", err)
266287
}
288+
if err := gzw.Close(); err != nil {
289+
t.Fatalf("closing gzip writer: %v", err)
290+
}
291+
return buf.Bytes()
292+
}
293+
294+
func TestExtractTarGzPathTraversal(t *testing.T) {
295+
t.Run("rejected paths", func(t *testing.T) {
296+
tests := []struct {
297+
name string
298+
entry string
299+
}{
300+
{"classic traversal", "../etc/passwd"},
301+
{"mid-path traversal", "foo/../../etc/passwd"},
302+
{"deep traversal", "foo/bar/../../../etc/shadow"},
303+
{"bare dotdot", ".."},
304+
{"trailing dotdot", "foo/.."},
305+
// foo/../bar is also rejected by the regex because it matches
306+
// `..` between slashes. This is intentionally conservative for
307+
// in-memory extraction where paths cannot be resolved.
308+
{"non-escaping dotdot", "foo/../bar"},
309+
}
310+
311+
for _, tt := range tests {
312+
t.Run(tt.name, func(t *testing.T) {
313+
data := createTarGz(t, map[string]string{tt.entry: "malicious"})
314+
_, err := ExtractTarGz(data)
315+
if err == nil {
316+
t.Errorf("expected error for path %q, got nil", tt.entry)
317+
}
318+
if err != nil && !strings.Contains(err.Error(), "invalid path") {
319+
t.Errorf("expected 'invalid path' error for %q, got: %v", tt.entry, err)
320+
}
321+
})
322+
}
323+
})
324+
325+
t.Run("accepted paths", func(t *testing.T) {
326+
entries := map[string]string{
327+
"safe/file.txt": "hello",
328+
"safe/nested/deep.txt": "world",
329+
}
330+
data := createTarGz(t, entries)
331+
files, err := ExtractTarGz(data)
332+
if err != nil {
333+
t.Fatalf("unexpected error for safe paths: %v", err)
334+
}
335+
336+
extracted := make(map[string]string)
337+
for _, f := range files {
338+
extracted[f.Name] = string(f.Data)
339+
}
340+
341+
for name, want := range entries {
342+
got, ok := extracted[name]
343+
if !ok {
344+
t.Errorf("missing extracted file %q", name)
345+
continue
346+
}
347+
if got != want {
348+
t.Errorf("file %q: got %q, want %q", name, got, want)
349+
}
350+
}
351+
})
352+
353+
t.Run("empty input", func(t *testing.T) {
354+
_, err := ExtractTarGz([]byte{})
355+
if err == nil {
356+
t.Error("expected error for empty input")
357+
}
358+
})
267359
}
268360

269361
func TestSanitizeFilename(t *testing.T) {

‎internal/manifest/manifest_test.go‎

Lines changed: 103 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package manifest
22

33
import (
4+
"archive/tar"
45
"bytes"
56
"compress/gzip"
67
"os"
@@ -81,28 +82,112 @@ func TestArchiveNotDirectory(t *testing.T) {
8182
}
8283
}
8384

84-
func TestExtractPathTraversal(t *testing.T) {
85-
// This test ensures the extract function rejects path traversal attacks
86-
// We can't easily create a malicious tar, but we test that normal paths work
87-
srcDir := t.TempDir()
88-
testDir := filepath.Join(srcDir, "safe")
89-
if err := os.MkdirAll(testDir, 0755); err != nil {
90-
t.Fatal(err)
91-
}
92-
if err := os.WriteFile(filepath.Join(testDir, "file.txt"), []byte("safe"), 0644); err != nil {
93-
t.Fatal(err)
94-
}
95-
85+
// createTarGzBytes builds a tar.gz archive in memory with arbitrary entry names.
86+
// This allows crafting malicious archives for security testing.
87+
func createTarGzBytes(t *testing.T, entries map[string]string) []byte {
88+
t.Helper()
9689
var buf bytes.Buffer
97-
if _, err := Archive(&buf, testDir); err != nil {
98-
t.Fatal(err)
90+
gzw := gzip.NewWriter(&buf)
91+
tw := tar.NewWriter(gzw)
92+
93+
for name, content := range entries {
94+
if err := tw.WriteHeader(&tar.Header{
95+
Name: name,
96+
Size: int64(len(content)),
97+
Mode: 0644,
98+
Typeflag: tar.TypeReg,
99+
}); err != nil {
100+
t.Fatalf("writing tar header for %q: %v", name, err)
101+
}
102+
if _, err := tw.Write([]byte(content)); err != nil {
103+
t.Fatalf("writing tar content for %q: %v", name, err)
104+
}
99105
}
100106

101-
dstDir := t.TempDir()
102-
_, err := Extract(&buf, dstDir)
103-
if err != nil {
104-
t.Fatalf("extract: %v", err)
107+
// Close tar then gzip explicitly (not defer) to ensure full flush.
108+
if err := tw.Close(); err != nil {
109+
t.Fatalf("closing tar writer: %v", err)
105110
}
111+
if err := gzw.Close(); err != nil {
112+
t.Fatalf("closing gzip writer: %v", err)
113+
}
114+
return buf.Bytes()
115+
}
116+
117+
func TestExtractPathTraversal(t *testing.T) {
118+
t.Run("rejected paths", func(t *testing.T) {
119+
tests := []struct {
120+
name string
121+
entry string
122+
}{
123+
{"direct traversal", "../escape.txt"},
124+
{"relative traversal", "subdir/../../escape.txt"},
125+
{"deep traversal", "foo/bar/../../../etc/shadow"},
126+
{"bare dotdot", ".."},
127+
}
128+
129+
for _, tt := range tests {
130+
t.Run(tt.name, func(t *testing.T) {
131+
data := createTarGzBytes(t, map[string]string{tt.entry: "malicious"})
132+
destDir := t.TempDir()
133+
_, err := Extract(bytes.NewReader(data), destDir)
134+
if err == nil {
135+
t.Errorf("expected error for path %q, got nil", tt.entry)
136+
}
137+
if err != nil && !strings.Contains(err.Error(), "invalid path") {
138+
t.Errorf("expected 'invalid path' error for %q, got: %v", tt.entry, err)
139+
}
140+
141+
// Verify no files were written outside destDir.
142+
entries, _ := os.ReadDir(destDir)
143+
if len(entries) > 0 {
144+
t.Errorf("expected no files written for traversal path, found %d entries", len(entries))
145+
}
146+
})
147+
}
148+
})
149+
150+
t.Run("accepted safe path", func(t *testing.T) {
151+
data := createTarGzBytes(t, map[string]string{
152+
"manifest/safe.txt": "safe content",
153+
})
154+
destDir := t.TempDir()
155+
_, err := Extract(bytes.NewReader(data), destDir)
156+
if err != nil {
157+
t.Fatalf("unexpected error for safe path: %v", err)
158+
}
159+
160+
got, err := os.ReadFile(filepath.Join(destDir, "manifest", "safe.txt"))
161+
if err != nil {
162+
t.Fatalf("reading extracted file: %v", err)
163+
}
164+
if string(got) != "safe content" {
165+
t.Errorf("got %q, want %q", got, "safe content")
166+
}
167+
})
168+
169+
// filepath.Clean resolves "foo/../bar" to "bar" which stays within destDir,
170+
// so the HasPrefix check correctly allows it. This differs from the core
171+
// package's regex which rejects any path containing ".." — both behaviors
172+
// are correct for their context (file-based vs in-memory extraction).
173+
t.Run("non-escaping dotdot accepted", func(t *testing.T) {
174+
data := createTarGzBytes(t, map[string]string{
175+
"foo/../bar.txt": "resolved content",
176+
})
177+
destDir := t.TempDir()
178+
_, err := Extract(bytes.NewReader(data), destDir)
179+
if err != nil {
180+
t.Fatalf("unexpected error for non-escaping dotdot: %v", err)
181+
}
182+
183+
got, err := os.ReadFile(filepath.Join(destDir, "bar.txt"))
184+
if err != nil {
185+
t.Fatalf("reading extracted file: %v", err)
186+
}
187+
if string(got) != "resolved content" {
188+
t.Errorf("got %q, want %q", got, "resolved content")
189+
}
190+
})
106191
}
107192

108193
func TestCountFiles(t *testing.T) {

0 commit comments

Comments
 (0)