mirror of
https://github.com/anchore/syft.git
synced 2026-11-01 23:16:30 +01:00
fix: hold archive chains to the same bounds when scanning and decoding
Decoding SPDX, CycloneDX and syft-json rebuilt nested archive chains from whatever the document said, with no checks on entry names, depth or length. A small crafted document could forge archive boundaries or cost memory quadratic in its size (a 3 MB SPDX allocated ~14 GB). - decoders now hold names to the scan's `EntryName` rules, and chains to `MaxDepth` (16) and PATH_MAX in length - a file within a refused chain is dropped, rather than moved to a top-level path or merged into its outer archive - the scan holds itself to the same bounds, so its SBOMs always decode back: a configured `nested-archive-max-depth` past 16 stops at 16, and an entry whose chain would be too long is skipped and the archive reported as truncated Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user