mirror of
https://github.com/anchore/syft.git
synced 2026-10-30 14:02:26 +01:00
fix: cap archive entry path depth
Adding an entry checked `HasPath` on every prefix of its path, so a name with thousands of components did O(depth^2) hashing per entry before anything was charged. - entry names over 256 path components are refused and reported like other unsafe names - implied parent directories are found walking from the deepest parent up, stopping at the first one that already exists Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
This commit is contained in:
@@ -215,6 +215,19 @@ func TestResolver_extract_dataEdgeCases(t *testing.T) {
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("an entry name with too many path components is skipped and reported", func(t *testing.T) {
|
||||
atCap := strings.Repeat("a/", maxEntryNameDepth-1) + "f"
|
||||
data := zipBytes(t, map[string]string{
|
||||
atCap: "at the cap",
|
||||
strings.Repeat("b/", maxEntryNameDepth) + "f": "over the cap",
|
||||
})
|
||||
r, _ := extractBytes(t, data, "test.zip", nil)
|
||||
names := storedNames(t, r)
|
||||
assert.Contains(t, names, atCap)
|
||||
assert.Len(t, names, maxEntryNameDepth, "only the entry at the cap and its implied directories")
|
||||
assert.ErrorContains(t, r.Truncated, "path components")
|
||||
})
|
||||
|
||||
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",
|
||||
@@ -526,12 +539,12 @@ func TestExtract_entryCountBombIsBoundedByTheMemoryLimit(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestExtract_deepPathBombIsBoundedByTheMemoryLimit(t *testing.T) {
|
||||
// 100 entries whose 4 KiB names each imply 500 distinct directories would be 50,000 nodes; the
|
||||
// implied directories are charged as nodes, so the memory limit bounds them too
|
||||
// 100 entries whose names each imply 250 distinct directories would be 25,000 nodes; the implied
|
||||
// directories are charged as nodes, so the memory limit bounds them too
|
||||
files := map[string]string{}
|
||||
for i := range 100 {
|
||||
var b strings.Builder
|
||||
for j := range 500 {
|
||||
for j := range maxEntryNameDepth - 6 {
|
||||
fmt.Fprintf(&b, "%03d-%03d/", i, j)
|
||||
}
|
||||
b.WriteString("f")
|
||||
|
||||
@@ -109,13 +109,16 @@ func (r *Resolver) add(hdr tar.Header, content io.Reader) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// directories the path implies that do not exist yet are nodes this entry pays for
|
||||
// directories the path implies that do not exist yet are nodes this entry pays for. Walking from the
|
||||
// deepest parent up stops at the first that exists, since its ancestors exist too
|
||||
var newDirs []string
|
||||
for i := 1; i < len(entryPath); i++ {
|
||||
if entryPath[i] == '/' && !r.tree.HasPath(stereoscopeFile.Path(entryPath[:i])) {
|
||||
newDirs = append(newDirs, entryPath[:i])
|
||||
for i := strings.LastIndexByte(entryPath, '/'); i > 0; i = strings.LastIndexByte(entryPath[:i], '/') {
|
||||
if r.tree.HasPath(stereoscopeFile.Path(entryPath[:i])) {
|
||||
break
|
||||
}
|
||||
newDirs = append(newDirs, entryPath[:i])
|
||||
}
|
||||
slices.Reverse(newDirs)
|
||||
if !r.chargeIndex(approxIndexBytes(hdr) + int64(len(newDirs))*approxIndexBytesPerEntry) {
|
||||
return ErrDiskLimitReached
|
||||
}
|
||||
@@ -459,6 +462,10 @@ func (entryReader) Close() error { return nil }
|
||||
// so a longer name is a cheap way to inflate the index and no filesystem could hold it anyway.
|
||||
const maxEntryNameBytes = 4096
|
||||
|
||||
// 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.
|
||||
const maxEntryNameDepth = 256
|
||||
|
||||
// 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. A name refused as unsafe marks the resolver truncated.
|
||||
@@ -511,6 +518,9 @@ func entryName(name string) (string, error) {
|
||||
return "", fmt.Errorf("entry name %q contains a control character", name)
|
||||
}
|
||||
cleaned := strings.TrimPrefix(path.Clean("/"+name), "/")
|
||||
if depth := strings.Count(cleaned, "/") + 1; depth > maxEntryNameDepth {
|
||||
return "", fmt.Errorf("entry name of %d path components exceeds the %d component cap", depth, maxEntryNameDepth)
|
||||
}
|
||||
if strings.Contains(cleaned, file.ArchivePathSeparator) {
|
||||
return "", fmt.Errorf("entry name %q contains the archive path separator %q", name, file.ArchivePathSeparator)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user