diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go index ee5b20701..ed167b8fc 100644 --- a/pkg/bundle/bundle.go +++ b/pkg/bundle/bundle.go @@ -270,10 +270,10 @@ func Unpack(in io.Reader, targetPath string) (*UnpackDetails, error) { continue } - // #nosec G305 path traversal is checked by validating that the resulting path does not contain unexpected special elements + // #nosec G305 path traversal is checked by validating that the resulting path does not escape targetPath var target = filepath.Join(targetPath, header.Name) - if strings.Contains(target, "/../") { - return nil, fmt.Errorf("targetPath validation failed, path contains unexpected special elements") + if rel, err := filepath.Rel(targetPath, target); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return nil, fmt.Errorf("targetPath validation failed, %q escapes the target directory", header.Name) } switch header.Typeflag { diff --git a/pkg/bundle/bundle_test.go b/pkg/bundle/bundle_test.go index 265974fb7..75046d9b0 100644 --- a/pkg/bundle/bundle_test.go +++ b/pkg/bundle/bundle_test.go @@ -5,6 +5,8 @@ package bundle_test import ( + "archive/tar" + "bytes" "fmt" "log" "net/http/httptest" @@ -85,6 +87,41 @@ var _ = Describe("Bundle", func() { }) }) }) + + DescribeTable("should reject a tar entry that attempts to escape the target directory", + func(entryName string) { + withTempDir(func(outerDir string) { + // nested four levels deep so that the "../../../../etc/canary" + // payload still escapes targetDir while resolving to a path + // inside outerDir, not the host's real /etc + targetDir := filepath.Join(outerDir, "a", "b", "c", "target") + Expect(os.MkdirAll(targetDir, os.FileMode(0755))).To(Succeed()) + + // where the malicious entry would land if it were not rejected + escapedPath := filepath.Join(targetDir, entryName) + + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + Expect(tw.WriteHeader(&tar.Header{ + Name: entryName, + Typeflag: tar.TypeReg, + Mode: 0644, + Size: int64(len("owned")), + })).To(Succeed()) + _, err := tw.Write([]byte("owned")) + Expect(err).ToNot(HaveOccurred()) + Expect(tw.Close()).To(Succeed()) + + _, err = Unpack(&buf, targetDir) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("escapes the target directory")) + + Expect(escapedPath).ToNot(BeAnExistingFile()) + }) + }, + Entry("single level traversal", "../canary"), + Entry("deep traversal matching the reported issue", "../../../../etc/canary"), + ) }) Context("packing/pushing and pulling/unpacking", func() {