diff --git a/cmd/syft/internal/options/pkg.go b/cmd/syft/internal/options/pkg.go index 182fe961b..6d424e1c8 100644 --- a/cmd/syft/internal/options/pkg.go +++ b/cmd/syft/internal/options/pkg.go @@ -26,7 +26,7 @@ note: enabling this may result in a performance impact since all discovered comp note: this only applies to the java package cataloger, and is ignored when nested-archive-max-depth is not 0 (every archive is searched)`) descriptions.Add(&o.ExcludeBinaryOverlapByOwnership, `allows users to exclude synthetic binary packages from the sbom these packages are removed if an overlap with a non-synthetic package is found`) - descriptions.Add(&o.NestedArchiveMaxDepth, `maximum depth to recursively catalog nested archives (0 = disabled, -1 = as deep as 16 levels) + descriptions.Add(&o.NestedArchiveMaxDepth, `maximum depth to recursively catalog nested archives (0 = disabled, -1 = as deep as 16 levels, the most allowed) note: when enabled this replaces the java cataloger's own unbounded jar-in-jar recursion, so archives deeper than this are reported as unknowns rather than cataloged`) descriptions.Add(&o.NestedArchiveMaxMemoryBytes, `maximum bytes of nested archive content held in memory at once (0 = none, -1 = unbounded)`) descriptions.Add(&o.NestedArchiveMaxDiskBytes, `maximum bytes of nested archive content on disk at once (0 = none, -1 = unbounded) diff --git a/internal/archive/extract_test.go b/internal/archive/extract_test.go index 17b1e3fbd..de9c4bfa9 100644 --- a/internal/archive/extract_test.go +++ b/internal/archive/extract_test.go @@ -228,6 +228,18 @@ func TestResolver_extract_dataEdgeCases(t *testing.T) { assert.ErrorContains(t, r.Truncated, "path components") }) + t.Run("an entry whose chain is longer than a path is skipped and reported", func(t *testing.T) { + // the chain is the archive's own path, the separator, then the name + fits := strings.Repeat("a", MaxPathBytes-len("test-archive!/")) + data := zipBytes(t, map[string]string{ + fits: "at the cap", + fits + "b": "over the cap", + }) + r, _ := extractBytes(t, data, "test.zip", nil) + assert.Equal(t, []string{fits}, storedNames(t, r)) + assert.ErrorContains(t, r.Truncated, "archive chain") + }) + t.Run("an over-long entry name is skipped", func(t *testing.T) { data := zipBytes(t, map[string]string{ strings.Repeat("a/", maxEntryNameBytes) + "f": "deep", diff --git a/internal/archive/resolver.go b/internal/archive/resolver.go index c69b16b6b..7ec0dc305 100644 --- a/internal/archive/resolver.go +++ b/internal/archive/resolver.go @@ -487,7 +487,17 @@ func (entryReader) Close() error { return nil } // maxEntryNameBytes is the longest entry name stored: PATH_MAX. Every path component becomes a node, // so a longer name is a cheap way to inflate the index and no filesystem could hold it anyway. -const maxEntryNameBytes = 4096 +const maxEntryNameBytes = MaxPathBytes + +// MaxPathBytes is PATH_MAX, the longest whole archive chain a scan stores or a decoded document may build. +// Each nested file copies its container's chain, so without it a long name shared by many small entries +// costs memory quadratic in the document. +const MaxPathBytes = 4096 + +// MaxDepth is the deepest archives nest: a scan stops there whatever depth is configured, and a decoded +// document is held to it. No real nesting comes close, and a hostile archive (a zip quine, a recursive bomb) +// must still end. +const MaxDepth = 16 // maxEntryNameDepth is the most path components an entry name may have. Adding an entry costs work per // component, and no real archive nests anywhere near this deep. @@ -507,7 +517,7 @@ func (r *Resolver) entryHeader(f archives.FileInfo) (tar.Header, bool) { return tar.Header{}, false } - name, err := entryName(f.NameInArchive) + name, err := EntryName(f.NameInArchive) if err != nil { r.truncate(fmt.Errorf("an entry was skipped: %w", err)) return tar.Header{}, false @@ -515,6 +525,11 @@ func (r *Resolver) entryHeader(f archives.FileInfo) (tar.Header, bool) { if name == "" { return tar.Header{}, false } + // held to the chain length a decoder accepts back, so the SBOM round-trips + if chain := len(r.archiveCoordinates.FullNestedPath()) + len(file.ArchivePathSeparator) + len(name); chain > MaxPathBytes { + r.truncate(fmt.Errorf("an entry was skipped: its path of %d bytes within the archive chain exceeds the %d byte cap", chain, MaxPathBytes)) + return tar.Header{}, false + } hdr, err := tar.FileInfoHeader(f.FileInfo, f.LinkTarget) if err != nil { @@ -533,11 +548,12 @@ func (r *Resolver) 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, returning "" when it +// EntryName cleans an archive entry's name to a path relative to the archive root, returning "" when it // names nothing inside the archive. This is the trust boundary for names: every one ends up in FullPath // and the encoders, so a name that could forge an archive boundary ("!/") or a line in a line-oriented -// format (control characters, unicode line separators) is refused. -func entryName(name string) (string, error) { +// format (control characters, unicode line separators) is refused. Decoders building nested coordinates +// from a document hold its names to the same rules. +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) } diff --git a/internal/task/archive_tasks.go b/internal/task/archive_tasks.go index aebc7f90b..50e805204 100644 --- a/internal/task/archive_tasks.go +++ b/internal/task/archive_tasks.go @@ -42,7 +42,7 @@ const ArchiveCatalogerTaskName = "archive-cataloger" // unknowns.IncludeUnexpandedArchives it also reports archives in formats it does not extract. // // exclusions are the scan's exclusion patterns, applied inside each archive. Returns nil when archive -// cataloging is disabled (MaxDepth == 0; negative means as deep as unboundedArchiveDepth). +// cataloging is disabled (MaxDepth == 0; negative or beyond archive.MaxDepth means as deep as archive.MaxDepth). func NewArchiveCatalogerTask(cfg cataloging.ArchiveSearchConfig, unknowns cataloging.UnknownsConfig, pkgTasks, fileTasks []Task, exclusions []string) Task { return newArchiveCatalogerTask(cfg.MaxDepth, archiveLimits(cfg), unknowns.IncludeUnexpandedArchives, pkgTasks, fileTasks, exclusions) } @@ -138,10 +138,6 @@ func (c *archiveCataloger) logStats() { log.WithFields(fields...).Info("nested archive cataloging complete") } -// unboundedArchiveDepth is the depth a negative MaxDepth stops at. No real nesting comes close, and a -// hostile archive (a zip quine, a recursive bomb) must still end. -const unboundedArchiveDepth = 16 - // catalog processes every archive in the resolver, recursing into each up to maxDepth. func (c *archiveCataloger) catalog(ctx context.Context, resolver file.Resolver, depth int, builder sbomsync.Builder) error { return c.catalogCandidates(ctx, resolver, c.discoverArchives(resolver), depth, builder) @@ -149,8 +145,9 @@ func (c *archiveCataloger) catalog(ctx context.Context, resolver file.Resolver, func (c *archiveCataloger) catalogCandidates(ctx context.Context, resolver file.Resolver, candidates []archiveCandidate, depth int, builder sbomsync.Builder) error { maxDepth := c.maxDepth - if maxDepth < 0 { - maxDepth = unboundedArchiveDepth + // a deeper scan would write chains no decoder accepts back + if maxDepth < 0 || maxDepth > archive.MaxDepth { + maxDepth = archive.MaxDepth } if depth >= maxDepth { // java's own recursion is off while this task runs, so an archive left here is otherwise lost diff --git a/internal/task/archive_tasks_test.go b/internal/task/archive_tasks_test.go index 53cbe905b..1b4e6d35c 100644 --- a/internal/task/archive_tasks_test.go +++ b/internal/task/archive_tasks_test.go @@ -247,20 +247,25 @@ func Test_NewArchiveCatalogerTask_gating(t *testing.T) { } } -func Test_archiveCataloger_negativeDepthStopsAtTheCeiling(t *testing.T) { - // "unbounded" must still end, or a zip quine or recursive bomb runs forever +func Test_archiveCataloger_depthStopsAtTheCeiling(t *testing.T) { + // "unbounded" must still end, or a zip quine or recursive bomb runs forever, and a configured depth + // past the ceiling must too, or the SBOM holds chains no decoder accepts back chain := testutils.Zip(t, map[string][]byte{"deep.txt": []byte("deep")}) - for range unboundedArchiveDepth + 4 { + for range archive.MaxDepth + 4 { chain = testutils.Zip(t, map[string][]byte{"n.zip": chain}) } rootDir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(rootDir, "outer.zip"), chain, 0o600)) - var seen []string - tsk := newTestTask(t, cataloging.DefaultArchiveSearchConfig().WithMaxDepth(-1), capturingTask(t, &seen, nil)) - _ = tsk.Execute(context.Background(), dirTestResolver{dir: rootDir}, sbomsync.NewBuilder(newTestSBOM())) + for _, depth := range []int{-1, archive.MaxDepth + 2} { + t.Run(fmt.Sprint(depth), func(t *testing.T) { + var seen []string + tsk := newTestTask(t, cataloging.DefaultArchiveSearchConfig().WithMaxDepth(depth), capturingTask(t, &seen, nil)) + _ = tsk.Execute(context.Background(), dirTestResolver{dir: rootDir}, sbomsync.NewBuilder(newTestSBOM())) - assert.Len(t, seen, unboundedArchiveDepth) + assert.Len(t, seen, archive.MaxDepth) + }) + } } func Test_archiveCataloger_anArchiveIdenticalToAnAncestorIsNotEntered(t *testing.T) { diff --git a/syft/cataloging/archive_search.go b/syft/cataloging/archive_search.go index b30ca1555..a404e513b 100644 --- a/syft/cataloging/archive_search.go +++ b/syft/cataloging/archive_search.go @@ -23,7 +23,8 @@ type ArchiveSearchConfig struct { // IncludeUnindexedArchives indicates whether to search within unindexed archive files (e.g., .tar*). IncludeUnindexedArchives bool `yaml:"include-unindexed-archives" json:"include-unindexed-archives" mapstructure:"include-unindexed-archives"` - // MaxDepth is how many levels of nested archives to catalog: 0 disables it, negative goes as deep as 16 levels. + // MaxDepth is how many levels of nested archives to catalog: 0 disables it, and no setting goes deeper than 16 levels + // (negative goes that deep). MaxDepth int `yaml:"max-depth" json:"max-depth" mapstructure:"max-depth"` // MaxMemoryBytes bounds the archive content held in memory at once; content that does not fit is diff --git a/syft/format/common/spdxhelpers/to_syft_model.go b/syft/format/common/spdxhelpers/to_syft_model.go index 7f132547d..92dd5087d 100644 --- a/syft/format/common/spdxhelpers/to_syft_model.go +++ b/syft/format/common/spdxhelpers/to_syft_model.go @@ -443,7 +443,11 @@ func collectSyftFiles(s *sbom.SBOM, spdxIDMap map[string]any, doc *spdx.Document } for id, f := range files { - l := file.NewLocationFromCoordinates(nested.coordinates(id)) + c, ok := nested.coordinates(id) + if !ok { + continue + } + l := file.NewLocationFromCoordinates(c) spdxIDMap[string(id)] = l s.Artifacts.FileMetadata[l.Coordinates] = toFileMetadata(f) @@ -457,29 +461,41 @@ type nestedFiles[K comparable] struct { own map[K]file.Coordinates container map[K]K resolved map[K]file.Coordinates + refused map[K]bool } func newNestedFiles[K comparable]() nestedFiles[K] { - return nestedFiles[K]{own: map[K]file.Coordinates{}, container: map[K]K{}, resolved: map[K]file.Coordinates{}} + return nestedFiles[K]{own: map[K]file.Coordinates{}, container: map[K]K{}, resolved: map[K]file.Coordinates{}, refused: map[K]bool{}} } -func (n nestedFiles[K]) coordinates(id K) file.Coordinates { - if c, ok := n.resolved[id]; ok { - return c - } - c := n.own[id] - if container, ok := n.container[id]; ok { - // a containment cycle ends where it started - delete(n.container, id) - archive := n.coordinates(container) - c = file.Coordinates{ - RealPath: archive.RealPath, - FileSystemID: archive.FileSystemID, - FullPath: archive.FullNestedPath() + file.ArchivePathSeparator + strings.TrimPrefix(c.RealPath, "/"), +// coordinates reports false for a file within a chain a scan would refuse (a bad name, too deep, too long), and +// for everything within it. Such a file is dropped rather than moved, since no scan put it anywhere else. +func (n nestedFiles[K]) coordinates(id K) (file.Coordinates, bool) { + // walk up to a resolved or outermost file, then resolve back down, so a long chain costs no stack + chain := []K{id} + for { + next, ok := n.container[chain[len(chain)-1]] + if _, resolved := n.resolved[chain[len(chain)-1]]; resolved || !ok { + break } + // a containment cycle ends where it started + delete(n.container, chain[len(chain)-1]) + chain = append(chain, next) } - n.resolved[id] = c - return c + for i := len(chain) - 1; i >= 0; i-- { + if _, ok := n.resolved[chain[i]]; ok { + continue + } + c := n.own[chain[i]] + if i+1 < len(chain) { + container := chain[i+1] + nested, ok := internal.NestedCoordinates(n.resolved[container], c.RealPath) + n.refused[chain[i]] = !ok || n.refused[container] + c = nested + } + n.resolved[chain[i]] = c + } + return n.resolved[id], !n.refused[id] } func toFileDigests(f *spdx.File) (digests []file.Digest) { diff --git a/syft/format/common/spdxhelpers/to_syft_model_test.go b/syft/format/common/spdxhelpers/to_syft_model_test.go index 23a8177ca..971e8f4d3 100644 --- a/syft/format/common/spdxhelpers/to_syft_model_test.go +++ b/syft/format/common/spdxhelpers/to_syft_model_test.go @@ -2,6 +2,7 @@ package spdxhelpers import ( "reflect" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -12,6 +13,7 @@ import ( "github.com/stretchr/testify/require" "github.com/anchore/packageurl-go" + "github.com/anchore/syft/internal/archive" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" "github.com/anchore/syft/syft/pkg" @@ -795,3 +797,24 @@ func Test_skipsPackagesWithGeneratedFromRelationship(t *testing.T) { assert.NotNil(t, s.Artifacts.Packages.Package("1")) assert.Nil(t, s.Artifacts.Packages.Package("1-src")) } + +func Test_nestedFiles_longChainIsCapped(t *testing.T) { + // a document can claim any chain; it decodes no deeper than a scan goes, without recursing per level, and + // what is past that is dropped + n := newNestedFiles[int]() + const length = 100_000 + for i := range length { + n.own[i] = file.NewCoordinates("/a.jar", "") + if i > 0 { + n.container[i] = i - 1 + } + } + + for i := length - 1; i >= 0; i-- { + c, ok := n.coordinates(i) + assert.Equal(t, i <= archive.MaxDepth, ok, "%d", i) + if ok { + assert.Equal(t, "/a.jar"+strings.Repeat("!/a.jar", i), c.FullNestedPath()) + } + } +} diff --git a/syft/format/common/spdxhelpers/to_syft_model_v3.go b/syft/format/common/spdxhelpers/to_syft_model_v3.go index 3d0e91044..19f5f9a3f 100644 --- a/syft/format/common/spdxhelpers/to_syft_model_v3.go +++ b/syft/format/common/spdxhelpers/to_syft_model_v3.go @@ -356,7 +356,11 @@ func v3collectSyftFiles(s *sbom.SBOM, spdxMap ptrMap[any], doc *spdx.Document, s } for _, f := range files { - l := file.NewLocationFromCoordinates(nested.coordinates(ptrTo(f))) + c, ok := nested.coordinates(ptrTo(f)) + if !ok { + continue + } + l := file.NewLocationFromCoordinates(c) spdxMap.Set(f, l) s.Artifacts.FileMetadata[l.Coordinates] = v3toFileMetadata(f) diff --git a/syft/format/internal/coordinates.go b/syft/format/internal/coordinates.go new file mode 100644 index 000000000..48333d9c6 --- /dev/null +++ b/syft/format/internal/coordinates.go @@ -0,0 +1,74 @@ +package internal + +import ( + "strings" + + "github.com/anchore/syft/internal/archive" + "github.com/anchore/syft/syft/file" +) + +// NestedCoordinates returns the coordinates of the entry named name directly within the archive at container, +// reporting false for a name a scan would refuse as an entry name, for nesting deeper than a scan goes, or for +// a chain longer than archive.MaxPathBytes. A decoded document can then only build chains bounded in depth and +// length. +func NestedCoordinates(container file.Coordinates, name string) (file.Coordinates, bool) { + if nestingDepth(container) >= archive.MaxDepth { + return file.Coordinates{}, false + } + entry, err := archive.EntryName(name) + if err != nil || entry == "" { + return file.Coordinates{}, false + } + chain := container.FullNestedPath() + if len(chain)+len(file.ArchivePathSeparator)+len(entry) > archive.MaxPathBytes { + return file.Coordinates{}, false + } + return file.Coordinates{ + RealPath: container.RealPath, + FileSystemID: container.FileSystemID, + FullPath: chain + file.ArchivePathSeparator + entry, + }, true +} + +// NormalizeCoordinates returns decoded coordinates in the form a resolver builds them, reporting false for a +// FullPath that is not a chain NestedCoordinates could have built within RealPath. Such a file is dropped rather +// than collapsed onto RealPath, where it would merge into the outer archive's own entry. A FullPath merely equal +// to RealPath is cleared. +func NormalizeCoordinates(c file.Coordinates) (file.Coordinates, bool) { + if c.FullPath == "" || c.FullPath == c.RealPath { + c.FullPath = "" + return c, true + } + rest, ok := strings.CutPrefix(c.FullPath, c.RealPath+file.ArchivePathSeparator) + // counted before splitting, so a hostile chain costs nothing to refuse + if !ok || len(c.FullPath) > archive.MaxPathBytes || strings.Count(rest, file.ArchivePathSeparator) >= archive.MaxDepth { + return file.Coordinates{}, false + } + for _, entry := range strings.Split(rest, file.ArchivePathSeparator) { + if cleaned, err := archive.EntryName(entry); err != nil || cleaned == "" || cleaned != entry { + return file.Coordinates{}, false + } + } + return c, true +} + +// NormalizeLocations applies NormalizeCoordinates to each location, dropping those it refuses. +func NormalizeLocations(locations []file.Location) []file.Location { + var out []file.Location + for _, l := range locations { + if c, ok := NormalizeCoordinates(l.Coordinates); ok { + l.Coordinates = c + out = append(out, l) + } + } + return out +} + +// nestingDepth is how many archives deep the file at c is. +func nestingDepth(c file.Coordinates) int { + rest, ok := strings.CutPrefix(c.FullPath, c.RealPath) + if !ok { + return 0 + } + return strings.Count(rest, file.ArchivePathSeparator) +} diff --git a/syft/format/internal/coordinates_test.go b/syft/format/internal/coordinates_test.go new file mode 100644 index 000000000..09a2b1cb2 --- /dev/null +++ b/syft/format/internal/coordinates_test.go @@ -0,0 +1,45 @@ +package internal + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/anchore/syft/internal/archive" + "github.com/anchore/syft/syft/file" +) + +func TestNestedCoordinates(t *testing.T) { + war := file.NewCoordinates("/app.war", "layer") + + got, ok := NestedCoordinates(war, "/WEB-INF/lib/a.jar") + require.True(t, ok) + assert.Equal(t, file.Coordinates{RealPath: "/app.war", FileSystemID: "layer", FullPath: "/app.war!/WEB-INF/lib/a.jar"}, got) + + for _, name := range []string{"", "/", "a.jar!/BOOT-INF/lib/b.jar", "a\nb", strings.Repeat("a", 4097)} { + _, ok := NestedCoordinates(war, name) + assert.False(t, ok, "%q", name) + } + + // nesting stops as deep as a scan goes + c := war + for range archive.MaxDepth { + c, ok = NestedCoordinates(c, "a.jar") + require.True(t, ok) + } + _, ok = NestedCoordinates(c, "a.jar") + assert.False(t, ok) + + // so does a chain longer than a path, however it gets there: a long container name shared by many entries + // would otherwise cost memory quadratic in the document + long := file.NewCoordinates("/"+strings.Repeat("a", archive.MaxPathBytes), "layer") + _, ok = NestedCoordinates(long, "a.jar") + assert.False(t, ok) + c = war + for range 3 { + c, ok = NestedCoordinates(c, strings.Repeat("a", 2000)) + } + assert.False(t, ok) +} diff --git a/syft/format/internal/cyclonedxutil/helpers/decoder.go b/syft/format/internal/cyclonedxutil/helpers/decoder.go index a64fb9dae..ef7bc1080 100644 --- a/syft/format/internal/cyclonedxutil/helpers/decoder.go +++ b/syft/format/internal/cyclonedxutil/helpers/decoder.go @@ -8,6 +8,7 @@ import ( "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" + "github.com/anchore/syft/syft/format/internal" "github.com/anchore/syft/syft/linux" "github.com/anchore/syft/syft/pkg" "github.com/anchore/syft/syft/sbom" @@ -52,11 +53,13 @@ func collectFiles(components []cyclonedx.Component, container *file.Coordinates, if component.Type == cyclonedx.ComponentTypeFile { c := file.NewCoordinates("/"+strings.TrimPrefix(component.Name, "/"), "") if container != nil { - c = file.Coordinates{ - RealPath: container.RealPath, - FileSystemID: container.FileSystemID, - FullPath: container.FullNestedPath() + file.ArchivePathSeparator + strings.TrimPrefix(component.Name, "/"), + nested, ok := internal.NestedCoordinates(*container, component.Name) + if !ok { + // a chain a scan would refuse is dropped with everything in it, since no scan put it + // anywhere else + continue } + c = nested } coordinates = &c if s.Artifacts.FileMetadata == nil { diff --git a/syft/format/internal/cyclonedxutil/helpers/decoder_test.go b/syft/format/internal/cyclonedxutil/helpers/decoder_test.go index 5092c3ff0..49bd0f9a5 100644 --- a/syft/format/internal/cyclonedxutil/helpers/decoder_test.go +++ b/syft/format/internal/cyclonedxutil/helpers/decoder_test.go @@ -481,3 +481,27 @@ func Test_useBomRefOverDerivedSyftArtifactID(t *testing.T) { assert.NotEqual(t, "", pkgsWithoutID[0].ID()) } + +func Test_collectFiles_refusedChainIsDropped(t *testing.T) { + // a nested file a scan could not have produced is dropped with everything in it, not moved to the root + fileComponent := func(name string, children ...cyclonedx.Component) cyclonedx.Component { + c := cyclonedx.Component{Type: cyclonedx.ComponentTypeFile, Name: name} + if len(children) > 0 { + c.Components = &children + } + return c + } + s := &sbom.SBOM{} + collectFiles([]cyclonedx.Component{ + fileComponent("/app.war", + fileComponent("WEB-INF/lib/a.jar"), + fileComponent("b.jar!/forged.jar", fileComponent("inner.txt")), + ), + }, nil, s) + + var got []string + for c := range s.Artifacts.FileMetadata { + got = append(got, c.FullNestedPath()) + } + assert.ElementsMatch(t, []string{"/app.war", "/app.war!/WEB-INF/lib/a.jar"}, got) +} diff --git a/syft/format/syftjson/to_syft_model.go b/syft/format/syftjson/to_syft_model.go index a66db17d0..a10d5ca76 100644 --- a/syft/format/syftjson/to_syft_model.go +++ b/syft/format/syftjson/to_syft_model.go @@ -80,7 +80,10 @@ func toSyftFiles(files []model.File) sbom.Artifacts { } for _, f := range files { - coord := f.Location + coord, ok := internal.NormalizeCoordinates(f.Location) + if !ok { + continue + } if f.Metadata != nil { fm, err := safeFileModeConvert(f.Metadata.Mode) if err != nil { @@ -163,7 +166,7 @@ func toSyftLicenses(m []model.License) (p []pkg.License) { SPDXExpression: l.SPDXExpression, Type: l.Type, URLs: l.URLs, - Locations: file.NewLocationSet(l.Locations...), + Locations: file.NewLocationSet(internal.NormalizeLocations(l.Locations)...), Contents: l.Contents, }) } @@ -239,7 +242,9 @@ func toSyftRelationships(doc *model.Document, catalog *pkg.Collection, relations idMap[doc.Source.ID] = toSyftSource(doc.Source) for _, f := range doc.Files { - idMap[f.ID] = f.Location + if c, ok := internal.NormalizeCoordinates(f.Location); ok { + idMap[f.ID] = c + } } var out []artifact.Relationship @@ -346,7 +351,7 @@ func toSyftPackage(p model.Package, idAliases map[string]string) pkg.Package { Name: p.Name, Version: p.Version, FoundBy: p.FoundBy, - Locations: file.NewLocationSet(p.Locations...), + Locations: file.NewLocationSet(internal.NormalizeLocations(p.Locations)...), Licenses: pkg.NewLicenseSet(toSyftLicenses(p.Licenses)...), Language: p.Language, Type: p.Type, diff --git a/syft/format/syftjson/to_syft_model_test.go b/syft/format/syftjson/to_syft_model_test.go index 6e4b4cc68..ccdbaec1d 100644 --- a/syft/format/syftjson/to_syft_model_test.go +++ b/syft/format/syftjson/to_syft_model_test.go @@ -5,6 +5,7 @@ import ( "io/fs" "math" "os" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -599,3 +600,39 @@ func Test_safeFileModeConvert(t *testing.T) { }) } } + +func Test_toSyftModel_fullPathIsNormalized(t *testing.T) { + plain := file.NewCoordinates("/app.jar", "") + nested := file.Coordinates{RealPath: "/app.war", FullPath: "/app.war!/lib/a.jar"} + decoded := []file.Coordinates{ + {RealPath: "/app.jar", FullPath: "/app.jar"}, + {RealPath: "/app.jar", FullPath: "/elsewhere!/app.jar"}, + // deeper than a scan goes + {RealPath: "/app.jar", FullPath: "/app.jar" + strings.Repeat("!/a.jar", 17)}, + // an entry name a scan would refuse + {RealPath: "/app.jar", FullPath: "/app.jar!/a\nb"}, + // longer than a path + {RealPath: "/app.jar", FullPath: "/app.jar!/" + strings.Repeat("a", 4096)}, + nested, + } + + var files []model.File + for _, c := range decoded { + files = append(files, model.File{ID: string(c.ID()), Location: c, Unknowns: []string{"x"}}) + } + got := toSyftFiles(files) + // a refused chain is dropped, not merged into the outer archive's own entry + assert.Equal(t, map[file.Coordinates][]string{plain: {"x"}, nested: {"x"}}, got.Unknowns) + + var locations []file.Location + for _, c := range decoded { + locations = append(locations, file.NewLocationFromCoordinates(c)) + } + p := toSyftPackage(model.Package{PackageBasicData: model.PackageBasicData{Locations: locations}}, nil) + var gotCoordinates []file.Coordinates + for _, l := range p.Locations.ToSlice() { + gotCoordinates = append(gotCoordinates, l.Coordinates) + } + // each location keeps its own access path, so only the coordinates are compared + assert.ElementsMatch(t, []file.Coordinates{plain, nested}, gotCoordinates) +}