fix: refuse archive entry names that forge paths or lines

`entryName` only rejected NUL and over-long names, so whatever an archive called an entry flowed
straight into `FullPath`, the file tree and every encoder.

- a name containing `!/` could pose as a nested archive boundary and collide with a real nested entry
- control characters and unicode line separators could add lines to SPDX tag-value output

Both are now refused, and refused names (over-long ones included) mark the resolver truncated with
the quoted name instead of being dropped silently.

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

internal/archive/README.md

@@ -89,6 +89,7 @@ files. `AllLocations` enumerates every non-directory path, links included. Answe
   archive task is running, so the java cataloger leaves nested archives to the task instead of
   opening them itself.
 - `entryHeader`: keeps regular files, directories and links only (no devices or fifos), cleans entry names to
-  archive-relative paths, refuses names longer than PATH_MAX or containing NUL, and drops header
+  archive-relative paths, refuses names longer than PATH_MAX or containing control characters, line
+  separators or the `!/` archive path separator (marking the resolver truncated), and drops header
   fields nothing reads (PAX records, user and group names, sub-second and access times) since the
   header is held for the life of the archive.

internal/archive/extract.go

@@ -140,7 +140,7 @@ func (r *Resolver) extract(ctx context.Context, format archives.Extractor, conte
 		return err
 	}
 	err := format.Extract(ctx, content, func(_ context.Context, f archives.FileInfo) error {
-		hdr, ok := entryHeader(f)
+		hdr, ok := r.entryHeader(f)
 		if !ok || exclusions.Excludes(hdr.Name) {
 			return nil
 		}

internal/archive/extract_test.go

@@ -225,6 +225,28 @@ func TestResolver_extract_dataEdgeCases(t *testing.T) {
 	})
 }
 
+func TestResolver_extract_unsafeEntryNamesAreRefusedAndReported(t *testing.T) {
+	jar := string(zipBytes(t, map[string]string{"META-INF/MANIFEST.MF": "real"}))
+	tests := []struct {
+		name   string
+		forged string
+	}{
+		{name: "archive path separator", forged: "a.jar!/META-INF/MANIFEST.MF"},
+		{name: "line break", forged: "x\nlayerID: sha256:forged"},
+		{name: "unicode line separator", forged: "x\u2028y"},
+		{name: "C1 control", forged: "x\u0085y"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			data := zipBytes(t, map[string]string{"a.jar": jar, tt.forged: "forged"})
+			r, _ := extractBytes(t, data, "outer.zip", nil)
+			assert.Equal(t, []string{"a.jar"}, storedNames(t, r))
+			require.Error(t, r.Truncated)
+			assert.Contains(t, r.Truncated.Error(), fmt.Sprintf("%q", tt.forged))
+		})
+	}
+}
+
 func TestResolver_extract_excludedEntriesAreNeverStored(t *testing.T) {
 	data := zipBytes(t, map[string]string{
 		"keep.txt":          "kept",

internal/archive/resolver.go

@@ -13,6 +13,7 @@ import (
 	"sort"
 	"strings"
 	"time"
+	"unicode"
 
 	"github.com/dustin/go-humanize"
 	"github.com/mholt/archives"
@@ -460,21 +461,24 @@ const maxEntryNameBytes = 4096
 
 // entryHeader builds the header stored for one archive entry, or reports false for an entry not worth
 // carrying: device nodes, fifos and sockets, which no cataloger reads, and names that cannot be a path
-// inside the archive.
+// inside the archive. A name refused as unsafe marks the resolver truncated.
 //
 // The name is cleaned to an archive-relative path with no leading slash, so "../x" names x at the
 // archive root rather than anything outside it. Symlinks are stored as headers, never followed on the
 // host; the resolver resolves their targets inside the archive.
-func entryHeader(f archives.FileInfo) (tar.Header, bool) {
+func (r *Resolver) entryHeader(f archives.FileInfo) (tar.Header, bool) {
 	mode := f.Mode()
 	if !mode.IsRegular() && !mode.IsDir() && mode.Type()&fs.ModeSymlink == 0 {
 		log.WithFields("entry", f.NameInArchive, "mode", mode).Debug("skipping non-regular archive entry")
 		return tar.Header{}, false
 	}
 
-	name, ok := entryName(f.NameInArchive)
+	name, err := entryName(f.NameInArchive)
-	if !ok {
+	if err != nil {
-		log.WithFields("entry-name-bytes", len(f.NameInArchive)).Trace("skipping archive entry with an unusable name")
+		r.truncate(fmt.Errorf("an entry was skipped: %w", err))
+		return tar.Header{}, false
+	}
+	if name == "" {
 		return tar.Header{}, false
 	}
 
@@ -495,12 +499,24 @@ func entryHeader(f archives.FileInfo) (tar.Header, bool) {
 	return *hdr, true
 }
 
-// entryName cleans an archive entry's name to a path relative to the archive root, reporting false
+// entryName cleans an archive entry's name to a path relative to the archive root, returning "" when it
-// when it names nothing inside the archive.
+// names nothing inside the archive. This is the trust boundary for names: every one ends up in FullPath
-func entryName(name string) (string, bool) {
+// and the encoders, so a name that could forge an archive boundary ("!/") or a line in a line-oriented
-	if len(name) > maxEntryNameBytes || strings.ContainsRune(name, 0) {
+// format (control characters, unicode line separators) is refused.
-		return "", false
+func entryName(name string) (string, error) {
+	if len(name) > maxEntryNameBytes {
+		return "", fmt.Errorf("entry name of %d bytes exceeds the %d byte cap", len(name), maxEntryNameBytes)
+	}
+	if strings.IndexFunc(name, isUnsafeNameRune) >= 0 {
+		return "", fmt.Errorf("entry name %q contains a control character", name)
 	}
 	cleaned := strings.TrimPrefix(path.Clean("/"+name), "/")
-	return cleaned, cleaned != ""
+	if strings.Contains(cleaned, file.ArchivePathSeparator) {
+		return "", fmt.Errorf("entry name %q contains the archive path separator %q", name, file.ArchivePathSeparator)
+	}
+	return cleaned, nil
+}
+
+func isUnsafeNameRune(c rune) bool {
+	return unicode.IsControl(c) || c == '\u2028' || c == '\u2029'
 }