mirror of
https://github.com/anchore/syft.git
synced 2026-10-11 21:57:22 +02:00
fix(debian): bound the decompressed members of a .deb archive (#5293)
This commit is contained in:
@@ -285,6 +285,7 @@ func TestDpkgArchiveCataloger(t *testing.T) {
|
||||
pkgtest.NewCatalogTester().
|
||||
WithImageResolver(t, tt.name).
|
||||
IgnoreLocationLayer(). // this fixture can be rebuilt, thus the layer ID will change
|
||||
WithErrorAssertion(require.NoError).
|
||||
Expects(tt.expected, nil).
|
||||
TestCataloger(t, c)
|
||||
})
|
||||
|
||||
@@ -202,7 +202,11 @@ func getAdditionalFileListing(resolver file.Resolver, dbLocation file.Location,
|
||||
if md5Reader != nil && md5Location != nil {
|
||||
defer internal.CloseAndLogError(md5Reader, md5Location.AccessPath)
|
||||
// attach the file list
|
||||
files = append(files, parseDpkgMD5Info(md5Reader)...)
|
||||
records, err := parseDpkgMD5Info(md5Reader)
|
||||
if err != nil {
|
||||
log.Debugf("dpkg md5sums file %q: %v", md5Location.RealPath, err)
|
||||
}
|
||||
files = append(files, records...)
|
||||
|
||||
// keep a record of the file where this was discovered
|
||||
locations = append(locations, *md5Location)
|
||||
@@ -213,7 +217,11 @@ func getAdditionalFileListing(resolver file.Resolver, dbLocation file.Location,
|
||||
if conffilesReader != nil && conffilesLocation != nil {
|
||||
defer internal.CloseAndLogError(conffilesReader, conffilesLocation.AccessPath)
|
||||
// attach the file list
|
||||
files = append(files, parseDpkgConffileInfo(conffilesReader)...)
|
||||
records, err := parseDpkgConffileInfo(conffilesReader)
|
||||
if err != nil {
|
||||
log.Debugf("dpkg conffiles file %q: %v", conffilesLocation.RealPath, err)
|
||||
}
|
||||
files = append(files, records...)
|
||||
|
||||
// keep a record of the file where this was discovered
|
||||
locations = append(locations, *conffilesLocation)
|
||||
|
||||
@@ -3,8 +3,10 @@ package debian
|
||||
import (
|
||||
"archive/tar"
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"math"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strings"
|
||||
@@ -13,6 +15,7 @@ import (
|
||||
"github.com/mholt/archives"
|
||||
|
||||
"github.com/anchore/syft/internal"
|
||||
intFile "github.com/anchore/syft/internal/file"
|
||||
"github.com/anchore/syft/internal/unknown"
|
||||
"github.com/anchore/syft/syft/artifact"
|
||||
"github.com/anchore/syft/syft/file"
|
||||
@@ -33,6 +36,7 @@ func parseDebArchive(ctx context.Context, _ file.Resolver, _ *generic.Environmen
|
||||
var metadata *pkg.DpkgArchiveEntry
|
||||
var licenses []string
|
||||
var unknownErr error
|
||||
var sawControl, sawData bool
|
||||
for {
|
||||
header, err := arReader.Next()
|
||||
if err == io.EOF {
|
||||
@@ -43,19 +47,28 @@ func parseDebArchive(ctx context.Context, _ file.Resolver, _ *generic.Environmen
|
||||
}
|
||||
|
||||
switch {
|
||||
case strings.HasPrefix(header.Name, "control.tar"):
|
||||
// a .deb has exactly one control.tar.* and one data.tar.* member; anything past the first of
|
||||
// each is attacker-supplied filler that would otherwise buy a fresh decompression budget
|
||||
case !sawControl && strings.HasPrefix(header.Name, "control.tar"):
|
||||
sawControl = true
|
||||
// Decompress the control.tar.* file
|
||||
dcReader, err := decompressionStream(ctx, arReader, header.Name)
|
||||
dcReader, err := decompressionStream(ctx, arReader, header.Name, maxControlTarSize)
|
||||
if err != nil {
|
||||
return nil, nil, unknown.New(reader.Location, fmt.Errorf("failed to decompress control.tar.* file: %w", err))
|
||||
}
|
||||
metadata, err = processControlTar(dcReader)
|
||||
if err != nil {
|
||||
switch {
|
||||
case err != nil && metadata == nil:
|
||||
return nil, nil, unknown.New(reader.Location, fmt.Errorf("failed to process control.tar.* file: %w", err))
|
||||
case err != nil:
|
||||
// metadata came back usable but incomplete (a clipped file listing); keep the package and
|
||||
// record why it is partial
|
||||
unknownErr = unknown.Append(unknownErr, reader.Location, fmt.Errorf("partial control.tar.* file: %w", err))
|
||||
}
|
||||
case strings.HasPrefix(header.Name, "data.tar"):
|
||||
case !sawData && strings.HasPrefix(header.Name, "data.tar"):
|
||||
sawData = true
|
||||
// Decompress the data.tar.* file
|
||||
dcReader, err := decompressionStream(ctx, arReader, header.Name)
|
||||
dcReader, err := decompressionStream(ctx, arReader, header.Name, maxDataTarSize)
|
||||
if err != nil {
|
||||
return nil, nil, unknown.New(reader.Location, fmt.Errorf("failed to decompress data.tar.* file: %w", err))
|
||||
}
|
||||
@@ -64,15 +77,21 @@ func parseDebArchive(ctx context.Context, _ file.Resolver, _ *generic.Environmen
|
||||
unknownErr = unknown.Append(unknownErr, reader.Location, fmt.Errorf("failed to process data.tar.* file: %w", err))
|
||||
}
|
||||
}
|
||||
|
||||
if sawControl && sawData {
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if metadata == nil {
|
||||
return nil, nil, unknown.New(reader.Location, fmt.Errorf("no application found described in .dpkg archive"))
|
||||
}
|
||||
|
||||
// a partial parse still yields a usable package, so report what went wrong alongside it rather than
|
||||
// dropping either one
|
||||
return []pkg.Package{
|
||||
newDebArchivePackage(ctx, reader.Location, *metadata, licenses),
|
||||
}, nil, nil
|
||||
}, nil, unknownErr
|
||||
}
|
||||
|
||||
// this is the pattern you'd expect to see in a tar header for a debian package license file ()
|
||||
@@ -81,6 +100,7 @@ var archiveHeaderLicensePathPattern = regexp.MustCompile(`^\.?/usr/share/doc/[^/
|
||||
func processDataTar(dcReader io.ReadCloser) ([]string, error) {
|
||||
defer internal.CloseAndLogError(dcReader, "")
|
||||
var licenses []string
|
||||
var copyrightFiles int
|
||||
|
||||
tarReader := tar.NewReader(dcReader)
|
||||
for {
|
||||
@@ -96,6 +116,10 @@ func processDataTar(dcReader io.ReadCloser) ([]string, error) {
|
||||
// TODO: in the future we can add archive sub indexes to the locations to see where within
|
||||
// the dpkg archive the license was found
|
||||
if archiveHeaderLicensePathPattern.MatchString(header.Name) {
|
||||
copyrightFiles++
|
||||
if copyrightFiles > maxCopyrightFiles {
|
||||
return licenses, fmt.Errorf("copyright file count exceeds %d; remaining files not read", maxCopyrightFiles)
|
||||
}
|
||||
licenses = append(licenses, parseLicensesFromCopyright(tarReader)...)
|
||||
}
|
||||
}
|
||||
@@ -103,6 +127,10 @@ func processDataTar(dcReader io.ReadCloser) ([]string, error) {
|
||||
return licenses, nil
|
||||
}
|
||||
|
||||
// processControlTar always returns whatever metadata it managed to parse, even alongside a non-nil
|
||||
// error. A non-nil error with non-nil metadata means the package is usable but incomplete (e.g. a
|
||||
// clipped file listing); nil metadata means nothing usable was found and the caller should treat it
|
||||
// as fatal.
|
||||
func processControlTar(dcReader io.ReadCloser) (*pkg.DpkgArchiveEntry, error) {
|
||||
defer internal.CloseAndLogError(dcReader, "")
|
||||
|
||||
@@ -111,6 +139,7 @@ func processControlTar(dcReader io.ReadCloser) (*pkg.DpkgArchiveEntry, error) {
|
||||
var metadata *pkg.DpkgArchiveEntry
|
||||
var files []pkg.DpkgFileRecord
|
||||
var confFileRecords []pkg.DpkgFileRecord
|
||||
var listingErr error
|
||||
|
||||
for {
|
||||
header, err := tarReader.Next()
|
||||
@@ -118,7 +147,7 @@ func processControlTar(dcReader io.ReadCloser) (*pkg.DpkgArchiveEntry, error) {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to read control tar: %w", err)
|
||||
return metadata, fmt.Errorf("failed to read control tar: %w", err)
|
||||
}
|
||||
|
||||
switch filepath.Base(header.Name) {
|
||||
@@ -134,11 +163,15 @@ func processControlTar(dcReader io.ReadCloser) (*pkg.DpkgArchiveEntry, error) {
|
||||
entry := pkg.DpkgArchiveEntry(entries[0].toDpkgEntry())
|
||||
metadata = &entry
|
||||
case "md5sums":
|
||||
// parseDpkgMD5Info already streams via bufio.Scanner
|
||||
files = parseDpkgMD5Info(tarReader)
|
||||
// parseDpkgMD5Info streams via bufio.Scanner and reports its own clipping/scan errors
|
||||
var err error
|
||||
files, err = parseDpkgMD5Info(tarReader)
|
||||
listingErr = errors.Join(listingErr, err)
|
||||
case "conffiles":
|
||||
// parseDpkgConffileInfo already streams via bufio.Scanner
|
||||
confFileRecords = parseDpkgConffileInfo(tarReader)
|
||||
// parseDpkgConffileInfo streams via bufio.Scanner and reports its own clipping/scan errors
|
||||
var err error
|
||||
confFileRecords, err = parseDpkgConffileInfo(tarReader)
|
||||
listingErr = errors.Join(listingErr, err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -159,10 +192,36 @@ func processControlTar(dcReader io.ReadCloser) (*pkg.DpkgArchiveEntry, error) {
|
||||
}
|
||||
|
||||
metadata.Files = files
|
||||
return metadata, nil
|
||||
|
||||
// a clipped or unreadable file listing leaves the package usable, so hand both back and let the
|
||||
// caller decide
|
||||
return metadata, listingErr
|
||||
}
|
||||
|
||||
func decompressionStream(ctx context.Context, r io.Reader, filePath string) (io.ReadCloser, error) {
|
||||
// a .deb's member sizes are bounded by the archive, but what they decompress to is not: xz and zstd
|
||||
// both exceed gzip's ~1032:1, so a few KB member can expand without limit. The two members get
|
||||
// different budgets because they hold different things.
|
||||
const (
|
||||
// control.tar holds only metadata (control, md5sums, conffiles). Even a package shipping a hundred
|
||||
// thousand files lands around 12MB of md5sums text.
|
||||
maxControlTarSize int64 = 16 * intFile.MB
|
||||
|
||||
// data.tar is the package payload, which is legitimately large (texlive and cuda ship hundreds of
|
||||
// MB), so this is only here to stop the tar walk from spinning forever on a bomb. It mirrors the
|
||||
// perFileReadLimit already used for archive extraction in internal/file/copy.go.
|
||||
maxDataTarSize int64 = 2 * intFile.GB
|
||||
|
||||
// a real .deb ships exactly one copyright file per package.
|
||||
maxCopyrightFiles = 1000
|
||||
)
|
||||
|
||||
// sentinels so callers (and tests) can match with errors.Is instead of substring matching.
|
||||
var (
|
||||
errDecompressedTooLarge = errors.New("decompressed stream is larger than the max allowed size")
|
||||
errClippedFileListing = errors.New("file listing exceeds the max allowed entries")
|
||||
)
|
||||
|
||||
func decompressionStream(ctx context.Context, r io.Reader, filePath string, maxDecompressed int64) (io.ReadCloser, error) {
|
||||
format, stream, err := archives.Identify(ctx, filePath, r)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to identify compression format: %w", err)
|
||||
@@ -178,5 +237,45 @@ func decompressionStream(ctx context.Context, r io.Reader, filePath string) (io.
|
||||
return nil, fmt.Errorf("failed to create decompression reader: %w", err)
|
||||
}
|
||||
|
||||
return rc, nil
|
||||
return newBoundedReadCloser(rc, maxDecompressed), nil
|
||||
}
|
||||
|
||||
// boundedReadCloser fails the read past a byte budget rather than truncating.
|
||||
//
|
||||
// io.LimitedReader is the obvious thing to reach for here and is wrong for this: past its limit it
|
||||
// just stops returning bytes, and both consumers of this are tar readers, which read a truncated
|
||||
// stream as a clean end of archive. Together that emits a package silently missing files, which is
|
||||
// the exact failure the bound exists to catch. The signal is recoverable by checking
|
||||
// LimitedReader.N after the walk finishes (the same shape as safeCopy in internal/file/copy.go,
|
||||
// which counts bytes off io.Copy), but nothing obliges a caller to write that check and a missed
|
||||
// one is silent all over again. Erroring at the Read is the version that cannot be forgotten.
|
||||
//
|
||||
// net/http's MaxBytesReader has precisely these semantics and is stdlib, but reaching into the HTTP
|
||||
// package to parse a .deb leaves the next reader wondering what it is doing here.
|
||||
type boundedReadCloser struct {
|
||||
io.ReadCloser
|
||||
remaining int64
|
||||
limit int64
|
||||
}
|
||||
|
||||
func newBoundedReadCloser(rc io.ReadCloser, limit int64) *boundedReadCloser {
|
||||
// budget is one byte past the cap so a stream of exactly limit bytes still reaches EOF normally,
|
||||
// and only the byte after the cap trips the error. Clamp so a limit near math.MaxInt64 can't wrap
|
||||
// remaining negative and fail every read immediately.
|
||||
if limit < 0 || limit > math.MaxInt64-1 {
|
||||
limit = math.MaxInt64 - 1
|
||||
}
|
||||
return &boundedReadCloser{ReadCloser: rc, remaining: limit + 1, limit: limit}
|
||||
}
|
||||
|
||||
func (b *boundedReadCloser) Read(p []byte) (int, error) {
|
||||
if b.remaining <= 0 {
|
||||
return 0, fmt.Errorf("%w (%d bytes)", errDecompressedTooLarge, b.limit)
|
||||
}
|
||||
if int64(len(p)) > b.remaining {
|
||||
p = p[:b.remaining]
|
||||
}
|
||||
n, err := b.ReadCloser.Read(p)
|
||||
b.remaining -= int64(n)
|
||||
return n, err
|
||||
}
|
||||
|
||||
@@ -3,11 +3,22 @@ package debian
|
||||
import (
|
||||
"archive/tar"
|
||||
"bytes"
|
||||
"compress/gzip"
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"strings"
|
||||
"testing"
|
||||
"testing/iotest"
|
||||
|
||||
"github.com/blakesmith/ar"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/anchore/syft/internal/unknown"
|
||||
"github.com/anchore/syft/syft/file"
|
||||
"github.com/anchore/syft/syft/pkg/cataloger/generic"
|
||||
)
|
||||
|
||||
func TestProcessControlTar(t *testing.T) {
|
||||
@@ -85,3 +96,273 @@ func writeTarEntry(t *testing.T, tw *tar.Writer, name, content string) {
|
||||
_, err := tw.Write([]byte(content))
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func Test_boundedReadCloser(t *testing.T) {
|
||||
payload := []byte("0123456789")
|
||||
|
||||
t.Run("a stream at exactly the cap reads through whole", func(t *testing.T) {
|
||||
// the positive assertion is the point: it proves the one-byte-past budget does not truncate
|
||||
// at-cap content, which a bare "did not error" check would not catch
|
||||
r := newBoundedReadCloser(io.NopCloser(bytes.NewReader(payload)), int64(len(payload)))
|
||||
|
||||
got, err := io.ReadAll(r)
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, payload, got)
|
||||
})
|
||||
|
||||
t.Run("a stream past the cap errors rather than truncating", func(t *testing.T) {
|
||||
r := newBoundedReadCloser(io.NopCloser(bytes.NewReader(payload)), int64(len(payload)-1))
|
||||
|
||||
_, err := io.ReadAll(r)
|
||||
|
||||
require.ErrorIs(t, err, errDecompressedTooLarge)
|
||||
})
|
||||
|
||||
t.Run("the underlying read error is not masked", func(t *testing.T) {
|
||||
r := newBoundedReadCloser(io.NopCloser(iotest.ErrReader(errBoom)), 1024)
|
||||
|
||||
_, err := io.ReadAll(r)
|
||||
|
||||
require.ErrorIs(t, err, errBoom)
|
||||
})
|
||||
}
|
||||
|
||||
var errBoom = errors.New("boom")
|
||||
|
||||
func Test_decompressionStream_boundsDecompressedSize(t *testing.T) {
|
||||
// a highly compressible payload standing in for a decompression bomb: tiny compressed, large out
|
||||
var raw bytes.Buffer
|
||||
raw.Write(make([]byte, 512*1024))
|
||||
|
||||
var gz bytes.Buffer
|
||||
gw := gzip.NewWriter(&gz)
|
||||
_, err := gw.Write(raw.Bytes())
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, gw.Close())
|
||||
|
||||
require.Less(t, gz.Len(), 4096, "compressed payload should be tiny relative to what it expands to")
|
||||
|
||||
rc, err := decompressionStream(context.Background(), bytes.NewReader(gz.Bytes()), "control.tar.gz", 64*1024)
|
||||
require.NoError(t, err, "the bound is enforced on read, not on open")
|
||||
t.Cleanup(func() { _ = rc.Close() })
|
||||
|
||||
_, err = io.ReadAll(rc)
|
||||
|
||||
require.ErrorIs(t, err, errDecompressedTooLarge)
|
||||
}
|
||||
|
||||
func Test_processControlTar_rejectsABombedControlMember(t *testing.T) {
|
||||
// end to end through the real consumer: a tar reader treats a truncated stream as a clean end of
|
||||
// archive, so this asserts the bomb surfaces as an error instead of a package missing its files.
|
||||
// "control" parses fully before the md5sums stream dies, so this also covers the Fix C contract: a
|
||||
// control tar read failure must not drop the package that was already parsed.
|
||||
var tarBuf bytes.Buffer
|
||||
tw := tar.NewWriter(&tarBuf)
|
||||
writeTarEntry(t, tw, "control", "Package: test-package\nVersion: 1.0.0\n")
|
||||
writeTarEntry(t, tw, "md5sums", strings.Repeat("d41d8cd98f00b204e9800998ecf8427e usr/bin/x\n", 20000))
|
||||
require.NoError(t, tw.Close())
|
||||
|
||||
metadata, err := processControlTar(newBoundedReadCloser(io.NopCloser(bytes.NewReader(tarBuf.Bytes())), 4096))
|
||||
|
||||
require.ErrorIs(t, err, errDecompressedTooLarge)
|
||||
require.NotNil(t, metadata, "a control tar read failure after \"control\" was parsed must not drop the package")
|
||||
assert.Equal(t, "test-package", metadata.Package)
|
||||
}
|
||||
|
||||
func Test_processControlTar_clippedListingIsUsableButReported(t *testing.T) {
|
||||
// the record cap must not drop the package: metadata comes back usable AND an error explains why the
|
||||
// file list is short, so the caller can record it as an unknown instead of discarding the package
|
||||
var tarBuf bytes.Buffer
|
||||
tw := tar.NewWriter(&tarBuf)
|
||||
writeTarEntry(t, tw, "control", "Package: test-package\nVersion: 1.0.0\n")
|
||||
writeTarEntry(t, tw, "md5sums", strings.Repeat("d41d8cd98f00b204e9800998ecf8427e usr/bin/x\n", maxDpkgFileRecords+10))
|
||||
require.NoError(t, tw.Close())
|
||||
|
||||
metadata, err := processControlTar(io.NopCloser(bytes.NewReader(tarBuf.Bytes())))
|
||||
|
||||
require.Error(t, err, "clipping must be reported")
|
||||
require.ErrorIs(t, err, errClippedFileListing)
|
||||
require.NotNil(t, metadata, "the package must survive a clipped file listing")
|
||||
assert.Equal(t, "test-package", metadata.Package)
|
||||
assert.Len(t, metadata.Files, maxDpkgFileRecords)
|
||||
}
|
||||
|
||||
// buildDeb assembles a minimal .deb (an ar archive of debian-binary + control.tar.gz + data.tar.gz).
|
||||
func buildDeb(t *testing.T, controlTar, dataTar []byte) []byte {
|
||||
t.Helper()
|
||||
|
||||
gzipped := func(b []byte) []byte {
|
||||
var out bytes.Buffer
|
||||
w := gzip.NewWriter(&out)
|
||||
_, err := w.Write(b)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, w.Close())
|
||||
return out.Bytes()
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
aw := ar.NewWriter(&buf)
|
||||
require.NoError(t, aw.WriteGlobalHeader())
|
||||
|
||||
for _, m := range []struct {
|
||||
name string
|
||||
body []byte
|
||||
}{
|
||||
{"debian-binary", []byte("2.0\n")},
|
||||
{"control.tar.gz", gzipped(controlTar)},
|
||||
{"data.tar.gz", gzipped(dataTar)},
|
||||
} {
|
||||
require.NoError(t, aw.WriteHeader(&ar.Header{Name: m.name, Size: int64(len(m.body))}))
|
||||
_, err := aw.Write(m.body)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
return buf.Bytes()
|
||||
}
|
||||
|
||||
func Test_parseDebArchive_surfacesPartialParseAsUnknown(t *testing.T) {
|
||||
// guards a silent failure: unknownErr was accumulated and then dropped on return, so a data.tar that
|
||||
// failed to parse produced a clean-looking SBOM with no record that anything went wrong. The property
|
||||
// that actually matters is that the returned error is a *unknown.CoordinateError: that's what decides
|
||||
// between "recorded as an unknown" (this case) and "aborts the entire scan" (anything else), per
|
||||
// internal/task/executor.go and syft/create_sbom.go. A plain substring match on "data.tar" would pass
|
||||
// just as well if unknown.Append were swapped for errors.Join, which is the regression that matters.
|
||||
var controlTar bytes.Buffer
|
||||
ctw := tar.NewWriter(&controlTar)
|
||||
writeTarEntry(t, ctw, "control", "Package: test-package\nVersion: 1.0.0\nArchitecture: all\n")
|
||||
require.NoError(t, ctw.Close())
|
||||
|
||||
// a data.tar that is not a valid tar at all, so processDataTar errors
|
||||
dataTar := []byte(strings.Repeat("not a tar file at all", 100))
|
||||
|
||||
debBytes := buildDeb(t, controlTar.Bytes(), dataTar)
|
||||
|
||||
loc := file.NewVirtualLocation("/test.deb", "/test.deb")
|
||||
pkgs, _, err := parseDebArchive(context.Background(), nil, &generic.Environment{},
|
||||
file.LocationReadCloser{
|
||||
Location: loc,
|
||||
ReadCloser: io.NopCloser(bytes.NewReader(debBytes)),
|
||||
})
|
||||
|
||||
require.Len(t, pkgs, 1, "the package must still be emitted")
|
||||
assert.Equal(t, "test-package", pkgs[0].Name)
|
||||
require.Error(t, err, "a failed data.tar parse must not be swallowed")
|
||||
|
||||
coordinateErrors, remaining := unknown.ExtractCoordinateErrors(err)
|
||||
require.NoError(t, remaining, "no non-coordinate errors should remain in the chain")
|
||||
require.Len(t, coordinateErrors, 1)
|
||||
assert.Equal(t, loc.Coordinates, coordinateErrors[0].Coordinates)
|
||||
assert.Contains(t, coordinateErrors[0].Reason.Error(), "data.tar")
|
||||
}
|
||||
|
||||
func Test_parseDebArchive_controlTarBombRejectedByRealSizeBound(t *testing.T) {
|
||||
// end to end through the real entrypoint using the real maxControlTarSize (16MB), not a swapped-in
|
||||
// test value: nothing else in this suite exercises that constant at its actual call site. The bomb
|
||||
// entry is written before "control", so "control" is never reached and processControlTar returns nil
|
||||
// metadata alongside the error -- this exercises parseDebArchive's fatal branch (metadata == nil).
|
||||
var controlTar bytes.Buffer
|
||||
ctw := tar.NewWriter(&controlTar)
|
||||
writeTarEntry(t, ctw, "bomb", strings.Repeat("\x00", 20*1024*1024))
|
||||
writeTarEntry(t, ctw, "control", "Package: test-package\nVersion: 1.0.0\n")
|
||||
require.NoError(t, ctw.Close())
|
||||
|
||||
debBytes := buildDeb(t, controlTar.Bytes(), nil)
|
||||
require.Less(t, len(debBytes), 64*1024, "compressed .deb should stay tiny relative to what it expands to")
|
||||
|
||||
loc := file.NewVirtualLocation("/bomb.deb", "/bomb.deb")
|
||||
pkgs, _, err := parseDebArchive(context.Background(), nil, &generic.Environment{},
|
||||
file.LocationReadCloser{
|
||||
Location: loc,
|
||||
ReadCloser: io.NopCloser(bytes.NewReader(debBytes)),
|
||||
})
|
||||
|
||||
require.Empty(t, pkgs, "no package should be emitted when the control tar never parses")
|
||||
require.Error(t, err)
|
||||
|
||||
coordinateErrors, remaining := unknown.ExtractCoordinateErrors(err)
|
||||
require.NoError(t, remaining, "no non-coordinate errors should remain in the chain")
|
||||
require.Len(t, coordinateErrors, 1)
|
||||
require.ErrorIs(t, coordinateErrors[0].Reason, errDecompressedTooLarge)
|
||||
}
|
||||
|
||||
// arMember is one named, already-encoded ar entry body, for tests that need control over the raw
|
||||
// member list (order, duplicates) that buildDeb's fixed three-member shape does not allow.
|
||||
type arMember struct {
|
||||
name string
|
||||
body []byte
|
||||
}
|
||||
|
||||
// buildDebWithMembers is buildDeb generalized to an arbitrary member list.
|
||||
func buildDebWithMembers(t *testing.T, members []arMember) []byte {
|
||||
t.Helper()
|
||||
|
||||
var buf bytes.Buffer
|
||||
aw := ar.NewWriter(&buf)
|
||||
require.NoError(t, aw.WriteGlobalHeader())
|
||||
|
||||
for _, m := range members {
|
||||
require.NoError(t, aw.WriteHeader(&ar.Header{Name: m.name, Size: int64(len(m.body))}))
|
||||
_, err := aw.Write(m.body)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
return buf.Bytes()
|
||||
}
|
||||
|
||||
func Test_parseDebArchive_ignoresDuplicateArMembers(t *testing.T) {
|
||||
// a .deb has exactly one control.tar.* and one data.tar.*; a second data.tar.* member is
|
||||
// attacker-supplied filler that would otherwise buy a fresh decompression budget, and here would also
|
||||
// fail to parse as a tar at all -- so this proves the walk stops after the first of each kind rather
|
||||
// than merely surviving a well-formed duplicate.
|
||||
gzipped := func(b []byte) []byte {
|
||||
var out bytes.Buffer
|
||||
w := gzip.NewWriter(&out)
|
||||
_, err := w.Write(b)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, w.Close())
|
||||
return out.Bytes()
|
||||
}
|
||||
|
||||
var controlTar bytes.Buffer
|
||||
ctw := tar.NewWriter(&controlTar)
|
||||
writeTarEntry(t, ctw, "control", "Package: test-package\nVersion: 1.0.0\n")
|
||||
require.NoError(t, ctw.Close())
|
||||
|
||||
var firstDataTar bytes.Buffer
|
||||
dtw := tar.NewWriter(&firstDataTar)
|
||||
require.NoError(t, dtw.Close())
|
||||
|
||||
debBytes := buildDebWithMembers(t, []arMember{
|
||||
{"debian-binary", []byte("2.0\n")},
|
||||
// the duplicate sits before control.tar.gz on purpose: the loop cannot have stopped yet at that
|
||||
// point, so only the per-member guard can keep it from being read
|
||||
{"data.tar.gz", gzipped(firstDataTar.Bytes())},
|
||||
{"data.tar.gz", []byte("not a tar file at all")}, // not even gzipped; would error if ever read
|
||||
{"control.tar.gz", gzipped(controlTar.Bytes())},
|
||||
})
|
||||
|
||||
pkgs, _, err := parseDebArchive(context.Background(), nil, &generic.Environment{},
|
||||
file.LocationReadCloser{
|
||||
Location: file.NewVirtualLocation("/test.deb", "/test.deb"),
|
||||
ReadCloser: io.NopCloser(bytes.NewReader(debBytes)),
|
||||
})
|
||||
|
||||
require.NoError(t, err, "the second data.tar.* member must never be reached")
|
||||
require.Len(t, pkgs, 1)
|
||||
}
|
||||
|
||||
func Test_processDataTar_capsCopyrightFiles(t *testing.T) {
|
||||
// a real .deb ships exactly one copyright file per package; this asserts the walk stops and errors
|
||||
// rather than accumulating an unbounded number of license reads
|
||||
var tarBuf bytes.Buffer
|
||||
tw := tar.NewWriter(&tarBuf)
|
||||
for i := 0; i < maxCopyrightFiles+10; i++ {
|
||||
writeTarEntry(t, tw, fmt.Sprintf("/usr/share/doc/pkg%d/copyright", i), "License: MIT\n")
|
||||
}
|
||||
require.NoError(t, tw.Close())
|
||||
|
||||
_, err := processDataTar(io.NopCloser(bytes.NewReader(tarBuf.Bytes())))
|
||||
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
@@ -141,7 +141,12 @@ func (d *dpkgExtractedMetadata) toDpkgEntry() pkg.DpkgDBEntry {
|
||||
|
||||
// there may be an optional conffiles section that we should persist as files
|
||||
if d.Conffiles != "" {
|
||||
entry.Files = parseDpkgConffileInfo(strings.NewReader(d.Conffiles))
|
||||
var err error
|
||||
entry.Files, err = parseDpkgConffileInfo(strings.NewReader(d.Conffiles))
|
||||
if err != nil {
|
||||
// toDpkgEntry has no error return, a log line is all that's available here
|
||||
log.Debugf("dpkg status conffiles field for package %q: %v", d.Package, err)
|
||||
}
|
||||
}
|
||||
|
||||
if entry.Files == nil {
|
||||
|
||||
@@ -2,6 +2,7 @@ package debian
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"fmt"
|
||||
"io"
|
||||
"strings"
|
||||
|
||||
@@ -9,10 +10,27 @@ import (
|
||||
"github.com/anchore/syft/syft/pkg"
|
||||
)
|
||||
|
||||
func parseDpkgMD5Info(reader io.Reader) (findings []pkg.DpkgFileRecord) {
|
||||
// maxDpkgFileRecords bounds the file lists parsed out of md5sums and conffiles. A byte cap on the
|
||||
// enclosing stream is not enough on its own: the shortest line that still yields a record is 4 bytes,
|
||||
// so 16MB of control.tar buys millions of records.
|
||||
//
|
||||
// The number comes from measuring what a record actually retains, not from the struct size. A
|
||||
// DpkgFileRecord is 32 bytes, but a populated one also heap-allocates a file.Digest and three string
|
||||
// backing arrays: 121 bytes each with realistic Debian paths, 70 with minimal ones. At this bound that
|
||||
// is ~24MB per package, and the effective ceiling is that times cataloger parallelism (NumCPU*4).
|
||||
// The largest real Debian packages ship on the order of 10^4 files, so this leaves ~5x headroom.
|
||||
const maxDpkgFileRecords = 200_000
|
||||
|
||||
// parseDpkgMD5Info returns the records it managed to parse, plus a non-nil error when the record bound
|
||||
// was hit (wrapping errClippedFileListing) or the scanner failed partway through.
|
||||
func parseDpkgMD5Info(reader io.Reader) ([]pkg.DpkgFileRecord, error) {
|
||||
var findings []pkg.DpkgFileRecord
|
||||
scanner := bufio.NewScanner(reader)
|
||||
|
||||
for scanner.Scan() {
|
||||
if len(findings) >= maxDpkgFileRecords {
|
||||
return findings, fmt.Errorf("%w: dpkg md5sums listing exceeds %d entries", errClippedFileListing, maxDpkgFileRecords)
|
||||
}
|
||||
line := scanner.Text()
|
||||
fields := strings.SplitN(line, " ", 2)
|
||||
if len(fields) == 2 {
|
||||
@@ -29,13 +47,22 @@ func parseDpkgMD5Info(reader io.Reader) (findings []pkg.DpkgFileRecord) {
|
||||
})
|
||||
}
|
||||
}
|
||||
return findings
|
||||
if err := scanner.Err(); err != nil {
|
||||
return findings, fmt.Errorf("failed to scan dpkg md5sums: %w", err)
|
||||
}
|
||||
return findings, nil
|
||||
}
|
||||
|
||||
func parseDpkgConffileInfo(reader io.Reader) (findings []pkg.DpkgFileRecord) {
|
||||
// parseDpkgConffileInfo returns the records it managed to parse, plus a non-nil error when the record
|
||||
// bound was hit (wrapping errClippedFileListing) or the scanner failed partway through.
|
||||
func parseDpkgConffileInfo(reader io.Reader) ([]pkg.DpkgFileRecord, error) {
|
||||
var findings []pkg.DpkgFileRecord
|
||||
scanner := bufio.NewScanner(reader)
|
||||
|
||||
for scanner.Scan() {
|
||||
if len(findings) >= maxDpkgFileRecords {
|
||||
return findings, fmt.Errorf("%w: dpkg conffiles listing exceeds %d entries", errClippedFileListing, maxDpkgFileRecords)
|
||||
}
|
||||
line := strings.Trim(scanner.Text(), " \n")
|
||||
fields := strings.SplitN(line, " ", 2)
|
||||
|
||||
@@ -70,5 +97,8 @@ func parseDpkgConffileInfo(reader io.Reader) (findings []pkg.DpkgFileRecord) {
|
||||
findings = append(findings, record)
|
||||
}
|
||||
}
|
||||
return findings
|
||||
if err := scanner.Err(); err != nil {
|
||||
return findings, fmt.Errorf("failed to scan dpkg conffiles: %w", err)
|
||||
}
|
||||
return findings, nil
|
||||
}
|
||||
|
||||
@@ -1,10 +1,13 @@
|
||||
package debian
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/google/go-cmp/cmp"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/anchore/syft/syft/file"
|
||||
@@ -45,7 +48,8 @@ func TestMD5SumInfoParsing(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { require.NoError(t, f.Close()) })
|
||||
|
||||
actual := parseDpkgMD5Info(f)
|
||||
actual, err := parseDpkgMD5Info(f)
|
||||
require.NoError(t, err)
|
||||
|
||||
if diff := cmp.Diff(test.expected, actual); diff != "" {
|
||||
t.Errorf("unexpected md5 files (-want +got):\n%s", diff)
|
||||
@@ -79,7 +83,8 @@ func TestConffileInfoParsing(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { require.NoError(t, f.Close()) })
|
||||
|
||||
actual := parseDpkgConffileInfo(f)
|
||||
actual, err := parseDpkgConffileInfo(f)
|
||||
require.NoError(t, err)
|
||||
|
||||
if diff := cmp.Diff(test.expected, actual); diff != "" {
|
||||
t.Errorf("unexpected md5 files (-want +got):\n%s", diff)
|
||||
@@ -88,3 +93,49 @@ func TestConffileInfoParsing(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func Test_parseDpkgInfoFiles_boundRecordCount(t *testing.T) {
|
||||
// a byte cap on the enclosing stream cannot bound these on its own, since the shortest line that
|
||||
// still yields a record is a few bytes. exercises the real maxDpkgFileRecords const directly, since
|
||||
// the limit is no longer injectable.
|
||||
t.Run("md5sums stops at the record bound", func(t *testing.T) {
|
||||
buf := strings.Repeat("d41d8cd98f00b204e9800998ecf8427e usr/bin/x\n", maxDpkgFileRecords+10)
|
||||
|
||||
got, err := parseDpkgMD5Info(strings.NewReader(buf))
|
||||
|
||||
assert.Len(t, got, maxDpkgFileRecords)
|
||||
require.ErrorIs(t, err, errClippedFileListing)
|
||||
})
|
||||
|
||||
t.Run("conffiles stops at the record bound", func(t *testing.T) {
|
||||
buf := strings.Repeat("/etc/x.conf\n", maxDpkgFileRecords+10)
|
||||
|
||||
got, err := parseDpkgConffileInfo(strings.NewReader(buf))
|
||||
|
||||
assert.Len(t, got, maxDpkgFileRecords)
|
||||
require.ErrorIs(t, err, errClippedFileListing)
|
||||
})
|
||||
|
||||
t.Run("a listing under the bound is returned whole", func(t *testing.T) {
|
||||
// guards against the bound clipping legitimate input
|
||||
got, err := parseDpkgMD5Info(strings.NewReader(
|
||||
"d41d8cd98f00b204e9800998ecf8427e usr/bin/a\n" +
|
||||
"d41d8cd98f00b204e9800998ecf8427e usr/bin/b\n"))
|
||||
|
||||
require.NoError(t, err)
|
||||
require.Len(t, got, 2)
|
||||
assert.Equal(t, "/usr/bin/a", got[0].Path)
|
||||
assert.Equal(t, "/usr/bin/b", got[1].Path)
|
||||
})
|
||||
}
|
||||
|
||||
func Test_parseDpkgMD5Info_scannerError(t *testing.T) {
|
||||
// a single line longer than bufio.MaxScanTokenSize with no trailing newline trips the scanner's
|
||||
// internal error rather than yielding a clean (and silently incomplete) empty list
|
||||
line := strings.Repeat("a", bufio.MaxScanTokenSize+1)
|
||||
|
||||
got, err := parseDpkgMD5Info(strings.NewReader(line))
|
||||
|
||||
assert.Empty(t, got)
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user