chore: pin the bounds from the unbounded allocation work (#5298)

* build(deps): bump go-make to v0.8.1

Picks up golangci-lint 2.13.2 (built with go1.27) in place of 2.11.4 (go1.26).
The old build panics outright on a go1.27 GOROOT with "file requires newer Go
version go1.27", and an older mise-shim build fails the quieter way: it loses
type info and reports zero findings with exit 0, so a ruleguard run comes back
green without having evaluated anything.

Also folds in what the newer linter flags:

- `reflect.Ptr` -> `reflect.Pointer` in three files, applied by --fix
- `tw.CellFormatting{MergeMode: ...}` -> `tw.CellMerging{Mode: ...}` in the
  cataloger info table, for a staticcheck SA1019 deprecation

Note the deprecation text names `CellConfig.CellMerging.Mode`, but the field is
`CellConfig.Merging`; `CellMerging` is the type. That command path has no test
coverage, so the swap was checked by rendering the same hierarchical-merge table
both ways and diffing: byte identical.

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

* chore(lint): tune goconst for data tables

goconst got stricter in golangci-lint 2.13 and now counts strings in map
literals and struct fields, which lights up every lookup table in the repo. It
was already enabled, so this is main going red on a linter bump rather than
anything new in the tree: 101 findings, and the default threshold of 3 means a
vendor name appearing in three rows of a table reads as a magic constant.

Raising `min-occurrences` to 5 takes that to 14. The rest split by whether the
repeated string is data or an actual constant:

- `cpegenerate` is excluded by path. It maps artifact names to vendors,
  products, and maven group ids, and repeats strings 35, 83, even 293 times, so
  no threshold reaches it. Hoisting those would trade a readable table for
  indirection.
- the two SBOM fixture builders in `format/internal/testutil` get a
  function-level nolint each. The repeated names and versions are what the
  golden files assert against.
- `mysql-cluster` and `php` were genuine repeated literals and are now
  constants.

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

* chore(test): extract measureAlloc into a shared helper

The allocation-budget tests added during the unbounded allocation work grew a
copy of the same helper in every package they touched: four identical
`measureAlloc` definitions plus two more spelled out inline against
`runtime.MemStats`.

Now `testutils.MeasureAlloc`, at the repo root so `internal/spillbuf` can reach
it too. The doc comment carries the two things that were only recorded in some
of the copies: TotalAlloc is process-wide so callers must not use t.Parallel,
and a budget assertion only means something if the fixture would really blow
past it unguarded, so measure the unguarded path as well.

No behavior change, the body was identical in all six.

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

* chore(lint): add ruleguard rules for overflow and swallowed EOF

Two rules out of the recent unbounded allocation work (#5187, #5195, #5267,
#5291, #5292, #5293, #5294).

`noOverflowingBoundsCheck` flags `offset + size > limit` (and <, <=, >=) where
an operand is unsigned, neither addend is constant, and the operands are at
least 32 bits. Unsigned addition wraps, so an offset out of a file header can
carry the sum past the wrap point and pass a check it should have failed. The
fix is to subtract against the limit instead. Constant addends are excluded
because `pos+4 <= uint32(len(buf))` is the normal way to step a cursor, and the
32-bit floor drops nibbles widened for a comparison. Would have caught four
sites across #5195 and #5291.

`noSwallowedEOF` flags `return nil` / `return nil, nil` on an io.EOF check. The
caller gets a zero value it cannot tell from real data, and anything decoding a
fixed-size struct off it parses the padding as content. That is the shape behind
#5186.

Both decide from the expression alone, and writing the fix deletes the pattern,
so neither needs a suppression anywhere in the tree. That was the bar: rules for
the allocation and decompression classes were tried and dropped, because the
bound they look for lives in a preceding statement (or, for a decompressor
constructor, always downstream), so no correct implementation can silence them.
That would have meant ~15 permanent nolints on code that is already right. The
rules here that do work don't judge whether a bound exists, they route to a
wrapper that takes one: noDirectTempFiles, noDirectELFOpen.

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

* test: pin allocation budgets on the alpm, deb, and changelog bounds

The bounds added for #5292, #5293 and #5294 are asserted by the error they
return. That keeps passing if the bound is removed and something else rejects
the input later, which is the failure these were written for. Each of these
measures bytes instead, and proves the fixture is really a bomb first so a
fixture that quietly stops being one fails rather than passing.

- alpm mtree: 16MB of newlines, well inside the 64MB byte cap, costs 16.5GB
  uncapped against 238MB with maxMtreeLines. That 16.5GB is the measurement the
  line cap was added for and it was recorded only in the commit message. Note
  what the cap actually buys: 300k entries still cost a few hundred MB, per
  concurrent cataloger.
- deb members: 1.28GB against 392KB. Both arms read with io.ReadAll on purpose,
  since the stream itself never holds much whatever the cap says; what the cap
  protects is a consumer that buffers, and the real one is a tar reader building
  entries.
- snap changelog: 1.28GB against 175KB. This one has no explicit cap at all. It
  is held by bufio.Scanner refusing a token past MaxScanTokenSize, which is
  incidental, so switching to io.ReadAll or a bufio.Reader to pick up long lines
  would remove it silently.

Nothing added for the kernel module reader: its copy is already
io.Copy(tempFile, io.LimitReader(rc, max+1)), so peak disk is bounded by
construction and heap is streaming. A heap budget would assert the wrong
resource, and the three existing subtests cover the cap, at-cap fidelity, and
spill cleanup.

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

* test: pin allocation budgets on the ai and alpm byte caps

Follows the same pattern as the alpm line cap: measure bytes, and prove the
fixture is really a bomb first so one that quietly stops being a bomb fails
rather than passes.

- safetensors: 252MB against 2.6KB. The existing over-cap case deliberately
  omits the body, since the guard rejects on the declared length before reading
  anything. That is the right test for the guard but it would keep passing if
  the check moved after the read, so this one delivers the bytes.
- gguf: 268MB against 168MB. copyHeader io.Copy's whatever it is handed, so
  maxHeaderSize holds only because every caller wraps the reader first. This
  pins that the limit is where the bound lives, so dropping the wrapper at a
  call site fails here.
- alpm byte cap: 740MB against 132MB, the counterpart to the line cap budget.

Worth knowing for anyone writing the next one of these: a single enormous line
does not exercise the alpm byte cap. go-mtree runs its own bufio.Scanner and
refuses a token past 64KB, so that fixture fails as "token too long" before the
byte cap is consulted. The fixture here uses ordinary 1KB lines, enough of them
to pass 64MB while staying under the 300k line cap, so the byte cap is actually
the thing under test.

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

* test: assert bytes retained instead of allocated for gguf copyHeader

the bounded arm writes into a bytes.Buffer, whose doubling growth costs a
multiple of what it holds and lands on a different power of two depending on
the platform. it measured 168MB locally and 320MB on CI, past the budget.
buf.Len() states the same property deterministically.

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

---------

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
This commit is contained in:
Alex Goodman
2026-09-17 14:11:16 -04:00
committed by GitHub
parent 02ba369d13
commit 3325493831
29 changed files with 398 additions and 144 deletions
+10
View File
@@ -32,6 +32,10 @@ linters:
funlen:
lines: 70
statements: 50
goconst:
# the default of 3 turns every repeated literal in a lookup table or a test fixture into a finding.
# 5 keeps the ones worth hoisting without flagging data that reads better written out.
min-occurrences: 5
gocritic:
enabled-checks:
- deferInLoop
@@ -54,6 +58,12 @@ linters:
- linters:
- revive
text: "var-naming: avoid package names that conflict"
# cpegenerate is almost entirely lookup tables mapping artifact names to vendors, products, and
# maven group ids. The repeated strings are the data itself, so hoisting them to constants would
# replace a readable table with indirection for no gain.
- linters:
- goconst
path: syft/pkg/cataloger/internal/cpegenerate/
paths:
- third_party$
- builtin$
+3 -3
View File
@@ -2,11 +2,11 @@ module github.com/anchore/syft/.make
go 1.25.8
require github.com/anchore/go-make v0.8.0
require github.com/anchore/go-make v0.8.1
require (
github.com/bmatcuk/doublestar/v4 v4.10.0 // indirect
github.com/goccy/go-yaml v1.19.2 // indirect
golang.org/x/mod v0.37.0 // indirect
golang.org/x/sys v0.46.0 // indirect
golang.org/x/mod v0.40.0 // indirect
golang.org/x/sys v0.47.0 // indirect
)
+6 -6
View File
@@ -1,10 +1,10 @@
github.com/anchore/go-make v0.8.0 h1:3XAD4WNHIbqY4kwFyCQJ7kKiYLzSzjB0UwzDVwRqlUY=
github.com/anchore/go-make v0.8.0/go.mod h1:4M6TnArb5w693VyWsgr5dCWrk2BLNu/ed4JUcsrzS34=
github.com/anchore/go-make v0.8.1 h1:GAhPXR+9qVQhFB1pfVlHFOzxAt3dTDTUIA7Tp7nx9tg=
github.com/anchore/go-make v0.8.1/go.mod h1:xALbVsPsDoJnJxdkjPlxOLZV7GbAdT5Pv3gd0kP1Lz0=
github.com/bmatcuk/doublestar/v4 v4.10.0 h1:zU9WiOla1YA122oLM6i4EXvGW62DvKZVxIe6TYWexEs=
github.com/bmatcuk/doublestar/v4 v4.10.0/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
github.com/goccy/go-yaml v1.19.2 h1:PmFC1S6h8ljIz6gMRBopkjP1TVT7xuwrButHID66PoM=
github.com/goccy/go-yaml v1.19.2/go.mod h1:XBurs7gK8ATbW4ZPGKgcbrY1Br56PdM69F7LkFRi1kA=
golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ=
golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0=
golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw=
golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
golang.org/x/mod v0.40.0 h1:hUv+3cXcdRHz08UmSiOob7sadHig73uo5bkXxQ/tvUs=
golang.org/x/mod v0.40.0/go.mod h1:0/weTWkPWGBikyTWAX3dkjVztMmBA5hM0DH6BElSupE=
golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs=
golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
+1 -1
View File
@@ -349,7 +349,7 @@ func renderCatalogerInfoTable(_ *capabilities.Document, catalogers []capabilitie
})),
tablewriter.WithConfig(tablewriter.Config{
Row: tw.CellConfig{
Formatting: tw.CellFormatting{MergeMode: tw.MergeHierarchical},
Merging: tw.CellMerging{Mode: tw.MergeHierarchical},
Alignment: tw.CellAlignment{Global: tw.AlignLeft},
},
}),
+5 -13
View File
@@ -8,12 +8,12 @@ import (
"math/rand"
"os"
"path/filepath"
"runtime"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/anchore/syft/internal/testutils"
"github.com/anchore/syft/internal/tmpdir"
)
@@ -161,13 +161,9 @@ func TestSparsePlacementIsNotAnAllocationKnob(t *testing.T) {
b := newTestBuffer(t, WithMemLimit(1<<20))
mustWrite(t, b, 0, repeat('A', 64))
var before, after runtime.MemStats
runtime.GC()
runtime.ReadMemStats(&before)
allocated := testutils.MeasureAlloc(t, func() {
mustWrite(t, b, far, repeat('B', 64))
runtime.ReadMemStats(&after)
allocated := after.TotalAlloc - before.TotalAlloc
})
t.Logf("writing 64 bytes at offset %d allocated %d bytes", far, allocated)
assert.Less(t, allocated, uint64(1<<20),
"a far placement must cost its own bytes, not its offset")
@@ -714,15 +710,11 @@ func TestMemoryTierGrowsAmortized(t *testing.T) {
)
b := newTestBuffer(t, WithMemLimit(limit))
var before, after runtime.MemStats
runtime.GC()
runtime.ReadMemStats(&before)
allocated := testutils.MeasureAlloc(t, func() {
for off := int64(0); off < limit; off += chunk {
mustWrite(t, b, off, repeat('A', chunk))
}
runtime.ReadMemStats(&after)
allocated := after.TotalAlloc - before.TotalAlloc
})
t.Logf("filling a %d byte tier in %d byte chunks allocated %d bytes", limit, chunk, allocated)
assert.Less(t, allocated, uint64(4*limit),
"growth has to be amortized, not a fresh copy of the whole tier per write")
+31
View File
@@ -0,0 +1,31 @@
package testutils
import (
"runtime"
"testing"
)
// MeasureAlloc reports the bytes allocated while fn ran.
//
// This exists for guards whose whole point is "a small input cannot make us reserve a large buffer".
// Only a byte count states that property: asserting that an error comes back would keep passing if the
// allocation were hoisted above the check, which is the bug these guards were written for.
//
// Two things to know when using it:
//
// - TotalAlloc is process-wide, so a test calling this must not call t.Parallel. Another test's
// allocations would land in the measurement.
// - a budget assertion is only meaningful if the fixture would really blow past it unguarded. Prefer
// measuring the unguarded path too and asserting it exceeds the declared size, so a fixture that
// quietly stops being a bomb fails instead of passing.
func MeasureAlloc(t testing.TB, fn func()) uint64 {
t.Helper()
var before, after runtime.MemStats
runtime.GC()
runtime.ReadMemStats(&before)
fn()
runtime.ReadMemStats(&after)
return after.TotalAlloc - before.TotalAlloc
}
@@ -93,7 +93,7 @@ func encode(out map[string]string, value reflect.Value, prefix string, fn FieldN
typ := value.Type()
switch typ.Kind() {
case reflect.Ptr:
case reflect.Pointer:
if value.IsNil() {
return
}
@@ -158,7 +158,7 @@ func fieldName(f reflect.StructField, prefix string, fn FieldName) (string, bool
// Decode based on the given type, applies all values to hydrate a new instance
func Decode(typ reflect.Type, values map[string]string, prefix string, fn FieldName) any {
isPtr := false
for typ.Kind() == reflect.Ptr {
for typ.Kind() == reflect.Pointer {
typ = typ.Elem()
isPtr = true
}
@@ -188,7 +188,7 @@ func Decode(typ reflect.Type, values map[string]string, prefix string, fn FieldN
func DecodeInto(obj any, values map[string]string, prefix string, fn FieldName) {
value := reflect.ValueOf(obj)
for value.Type().Kind() == reflect.Ptr {
for value.Type().Kind() == reflect.Pointer {
value = value.Elem()
}
@@ -205,7 +205,7 @@ func decode(vals map[string]string, value reflect.Value, prefix string, fn Field
incoming, valid := vals[prefix]
switch typ.Kind() {
case reflect.Ptr:
case reflect.Pointer:
t := typ.Elem()
v := value
if v.IsNil() {
@@ -269,13 +269,13 @@ func decode(vals map[string]string, value reflect.Value, prefix string, fn Field
str := fmt.Sprintf("%s:%d", prefix, idx)
// create new placeholder and decode values
newType := t
if t.Kind() == reflect.Ptr {
if t.Kind() == reflect.Pointer {
newType = t.Elem()
}
v := reflect.New(newType)
if decode(vals, v.Elem(), str, fn) {
// append to slice
if t.Kind() != reflect.Ptr {
if t.Kind() != reflect.Pointer {
v = v.Elem()
}
slice = reflect.Append(slice, v)
@@ -304,7 +304,7 @@ func decode(vals map[string]string, value reflect.Value, prefix string, fn Field
keyVals[key] = after
// create new placeholder and decode key
newKeyType := keyType
if keyType.Kind() == reflect.Ptr {
if keyType.Kind() == reflect.Pointer {
newKeyType = keyType.Elem()
}
k := reflect.New(newKeyType)
@@ -312,18 +312,18 @@ func decode(vals map[string]string, value reflect.Value, prefix string, fn Field
log.Debugf("unable to decode key for: %s", key)
continue
}
if keyType.Kind() != reflect.Ptr {
if keyType.Kind() != reflect.Pointer {
k = k.Elem()
}
// create new placeholder and decode value
newValueType := valueType
if valueType.Kind() == reflect.Ptr {
if valueType.Kind() == reflect.Pointer {
newValueType = valueType.Elem()
}
v := reflect.New(newValueType)
if decode(vals, v.Elem(), key, fn) {
if valueType.Kind() != reflect.Ptr {
if valueType.Kind() != reflect.Pointer {
v = v.Elem()
}
@@ -367,7 +367,7 @@ func PtrToStruct(ptr any) any {
return nil
}
switch v.Type().Kind() {
case reflect.Ptr:
case reflect.Pointer:
return PtrToStruct(v.Elem().Interface())
case reflect.Interface:
return PtrToStruct(v.Elem().Interface())
@@ -16,6 +16,7 @@ import (
"github.com/anchore/syft/syft/source/directorysource"
)
//nolint:goconst // fixture data: the repeated names and versions are what the golden files assert against, so they read better spelled out than behind constants
func DirectoryInput(t testing.TB, dir string) sbom.SBOM {
catalog := newDirectoryCatalog()
@@ -97,6 +98,7 @@ func DirectoryInputWithAuthorField(t testing.TB) sbom.SBOM {
}
}
//nolint:goconst // fixture data: the repeated names and versions are what the golden files assert against, so they read better spelled out than behind constants
func newDirectoryCatalog() *pkg.Collection {
catalog := pkg.NewCollection()
ctx := context.TODO()
@@ -19,6 +19,7 @@ import (
"github.com/anchore/syft/syft/source/stereoscopesource"
)
//nolint:goconst // fixture data: the repeated names and versions are what the golden files assert against, so they read better spelled out than behind constants
func ImageInput(t testing.TB, testImage string, options ...ImageOption) sbom.SBOM {
t.Helper()
catalog := pkg.NewCollection()
@@ -95,6 +96,7 @@ func changeToDirectoryWithGoldenFixture(t testing.TB, testImage string) func() {
return fn
}
//nolint:goconst // fixture data: the repeated names and versions are what the golden files assert against, so they read better spelled out than behind constants
func populateImageCatalog(catalog *pkg.Collection, img *image.Image) {
// TODO: this helper function is coupled to the image-simple fixture, which seems like a bad idea
_, ref1, _ := img.SquashedTree().File("/somefile-1.txt", filetree.FollowBasenameLinks)
+7 -9
View File
@@ -7,11 +7,12 @@ import (
"encoding/binary"
"fmt"
"io"
"runtime"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/anchore/syft/internal/testutils"
)
// section describes one section to place in a fixture. The compression headers are deliberately
@@ -543,13 +544,10 @@ func TestNewFile_BombDoesNotAllocate(t *testing.T) {
{name: ".strtab", typ: elf.SHT_STRTAB},
}, buildOpts{})
var before, after runtime.MemStats
runtime.GC()
runtime.ReadMemStats(&before)
_, err := NewFile(bytes.NewReader(data))
runtime.ReadMemStats(&after)
var err error
allocated := testutils.MeasureAlloc(t, func() {
_, err = NewFile(bytes.NewReader(data))
})
require.Error(t, err)
assert.Contains(t, err.Error(), "over the")
@@ -557,7 +555,7 @@ func TestNewFile_BombDoesNotAllocate(t *testing.T) {
// the guard has to reject before the allocation, not after it. The margin is loose on purpose: this
// is asserting "nothing near the declared size was allocated", not a precise budget.
const margin = 32 * 1024 * 1024
assert.Less(t, after.TotalAlloc-before.TotalAlloc, uint64(margin),
assert.Less(t, allocated, uint64(margin),
"parsing allocated far more than the input warrants")
}
@@ -13,6 +13,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/anchore/syft/internal/testutils"
"github.com/anchore/syft/syft/artifact"
"github.com/anchore/syft/syft/file"
"github.com/anchore/syft/syft/pkg"
@@ -1200,3 +1201,60 @@ func TestDockerAIModelConfigMediaTypes(t *testing.T) {
})
}
}
// TestReadSafeTensorsHeader_capBoundsHeap pins maxSafeTensorsHeaderSize as bytes rather than as an
// error string. The existing over-cap case omits the body precisely because the guard rejects before
// allocating, which means it would keep passing if the guard moved after the read. This one delivers
// the bytes, so moving the check shows up as heap.
func TestReadSafeTensorsHeader_capBoundsHeap(t *testing.T) {
const declared = maxSafeTensorsHeaderSize + 1
body := bytes.Repeat([]byte{'x'}, declared)
file := make([]byte, 8, 8+len(body))
binary.LittleEndian.PutUint64(file, uint64(declared))
file = append(file, body...)
// the unguarded shape: honor the declared length without weighing it against a ceiling first.
// This is what readSafeTensorsHeader does once the cap is passed, so it is what the cap prevents.
unbounded := testutils.MeasureAlloc(t, func() {
b, err := io.ReadAll(io.LimitReader(bytes.NewReader(file[8:]), declared))
require.NoError(t, err)
require.Len(t, b, declared)
})
require.Greater(t, unbounded, uint64(declared),
"fixture did not actually cost more than its own size; it is no longer a bomb")
bounded := testutils.MeasureAlloc(t, func() {
_, err := readSafeTensorsHeader(bytes.NewReader(file))
require.ErrorContains(t, err, "exceeds maximum")
})
t.Logf("unbounded allocated %d bytes, bounded allocated %d bytes", unbounded, bounded)
assert.Less(t, bounded, uint64(8*1024*1024),
"the declared-size check has to reject before the body is read, not after")
}
// TestCopyHeader_boundIsTheCallersLimitedReader pins where the GGUF bound actually lives. copyHeader
// io.Copy's whatever it is handed, so maxHeaderSize only holds because every caller wraps the reader
// first. A caller that stops doing that reads the whole file, and nothing in copyHeader would object.
//
// Bytes retained is asserted directly rather than through testutils.MeasureAlloc: the destination is a
// bytes.Buffer, whose doubling growth costs a multiple of what it holds and lands on a different power
// of two depending on the platform. buf.Len() is the same property without that noise.
func TestCopyHeader_boundIsTheCallersLimitedReader(t *testing.T) {
const payload = 2*maxHeaderSize + 1
file := make([]byte, 24, 24+payload)
binary.LittleEndian.PutUint32(file[0:4], ggufMagicNumber)
file = append(file, bytes.Repeat([]byte{'x'}, payload)...)
var unbounded bytes.Buffer
require.NoError(t, copyHeader(&unbounded, bytes.NewReader(file)))
require.Equal(t, len(file), unbounded.Len(),
"an unwrapped reader is copied whole; copyHeader applies no limit of its own")
var bounded bytes.Buffer
require.NoError(t, copyHeader(&bounded, &io.LimitedReader{R: bytes.NewReader(file), N: maxHeaderSize}))
assert.Equal(t, maxHeaderSize, bounded.Len(),
"a wrapped reader must cap the copy at maxHeaderSize")
}
@@ -12,9 +12,11 @@ import (
"time"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/internal/testutils"
"github.com/anchore/syft/syft/file"
"github.com/anchore/syft/syft/pkg"
)
@@ -484,3 +486,79 @@ func Test_parseMtree_malformedInput(t *testing.T) {
require.ErrorContains(t, err, "token too long")
})
}
// Test_parseMtree_lineCapBoundsHeap pins the measurement that maxMtreeLines exists for. The byte cap
// alone does not bound this path: go-mtree keeps an entry per line, so a listing well inside 64MB of
// bytes still costs multiples of its own size in heap. Asserting the error would keep passing if the
// line cap were removed, because the byte cap would still reject a large enough input eventually.
func Test_parseMtree_lineCapBoundsHeap(t *testing.T) {
// newline-only lines are the worst case: minimum bytes per entry, so the most entries per byte
const payloadSize = 16 * intFile.MB
spec := bytes.Repeat([]byte{'\n'}, payloadSize)
// well inside the byte cap, so only the line cap can stop this
require.Less(t, int64(payloadSize), int64(maxMtreeSize))
unbounded := testutils.MeasureAlloc(t, func() {
// the same listing with the line cap lifted, to prove the fixture is really a bomb
_, err := parseMtreeWithLimits(gzipOf(t, spec), maxMtreeSize, payloadSize+1)
t.Logf("unbounded parse err: %v", err)
})
require.Greater(t, unbounded, uint64(payloadSize),
"fixture did not actually cost more than its own size; it is no longer a bomb")
bounded := testutils.MeasureAlloc(t, func() {
_, err := parseMtreeWithLimits(gzipOf(t, spec), maxMtreeSize, maxMtreeLines)
require.ErrorIs(t, err, errTooManyMtreeLines)
})
t.Logf("unbounded allocated %d bytes, bounded allocated %d bytes", unbounded, bounded)
// measured at ~238MB against ~16.5GB uncapped, so the cap is doing its job, but note what it
// actually buys: 300k entries still cost a few hundred MB, and that is per concurrent cataloger.
// The budget is set to catch the cap being removed or raised by an order of magnitude, not to pin
// the exact figure.
assert.Less(t, bounded, uint64(512*intFile.MB),
"the line cap has to stop the parse before it builds an entry per line")
}
// Test_parseMtree_sizeCapBoundsHeap is the byte-cap counterpart to the line-cap budget above.
// Test_parseMtree_boundsDecompressedSize already covers the cap behaviorally, but with a spec small
// enough that buffering the whole listing would pass anyway. This one expands well past the cap, so a
// bound checked after the read instead of during it shows up as heap.
//
// The line length matters: go-mtree runs its own bufio.Scanner, which refuses a token past 64KB, so a
// single enormous line fails as "token too long" before the byte cap is ever consulted. Lines have to
// be ordinary for the byte cap to be the thing under test, and there have to be few enough of them
// that the line cap does not fire first.
func Test_parseMtree_sizeCapBoundsHeap(t *testing.T) {
const (
lineLen = 1024
numLines = 96 * 1024 // ~96MB across ~98k lines: past the 64MB cap, well under the 300k line cap
)
var raw bytes.Buffer
line := append(bytes.Repeat([]byte{'a'}, lineLen-1), '\n')
for range numLines {
raw.Write(line)
}
require.Greater(t, int64(raw.Len()), int64(maxMtreeSize), "fixture must exceed the byte cap")
require.Less(t, numLines, maxMtreeLines, "and must not trip the line cap first")
unbounded := testutils.MeasureAlloc(t, func() {
// the same listing with the byte cap lifted past it, to prove the fixture is really a bomb
_, err := parseMtreeWithLimits(gzipOf(t, raw.Bytes()), int64(raw.Len())*2, maxMtreeLines)
t.Logf("unbounded parse err: %v", err)
})
require.Greater(t, unbounded, uint64(raw.Len()),
"fixture did not actually cost more than its own size; it is no longer a bomb")
bounded := testutils.MeasureAlloc(t, func() {
_, err := parseMtreeWithLimits(gzipOf(t, raw.Bytes()), maxMtreeSize, maxMtreeLines)
require.ErrorIs(t, err, errMtreeTooLarge)
})
t.Logf("unbounded allocated %d bytes, bounded allocated %d bytes", unbounded, bounded)
assert.Less(t, bounded, uint64(512*intFile.MB),
"the size cap has to stop the read at the cap, not after the member is drained")
}
+8 -5
View File
@@ -8,6 +8,9 @@ import (
"github.com/anchore/syft/syft/pkg/cataloger/internal/binutils"
)
// mysqlClusterPackage is the package name shared by every MySQL Cluster classifier below.
const mysqlClusterPackage = "mysql-cluster"
// in both binaries and shared libraries, the version pattern is [NUL]3.11.2[NUL]
var pythonVersionTemplate = `(?m)\x00(?P<version>{{ .version }}[-._a-zA-Z0-9]*)\x00`
@@ -374,7 +377,7 @@ func DefaultClassifiers() []binutils.Classifier {
Class: "mysqld-mysql-cluster-legacy-binary",
EvidenceMatcher: m.FileContentsVersionMatcher(
`cluster-gpl\x00[0-9]+(\.[0-9]+)?(\.[0-9]+)?\-ndb\-(?P<version>[0-9]+(\.[0-9]+)?(\.[0-9]+)?)`),
Package: "mysql-cluster",
Package: mysqlClusterPackage,
PURL: mustPURL("pkg:generic/mysql-cluster@version"),
CPEs: []cpe.CPE{
cpe.Must("cpe:2.3:a:oracle:mysql_cluster:*:*:*:*:*:*:*:*", cpe.NVDDictionaryLookupSource),
@@ -385,7 +388,7 @@ func DefaultClassifiers() []binutils.Classifier {
Class: "mysqld-mysql-cluster-binary",
EvidenceMatcher: m.FileContentsVersionMatcher(
`/mysql-cluster-gpl-(?P<version>[0-9]+(\.[0-9]+)?(\.[0-9]+)?(alpha[0-9]|beta[0-9]|rc[0-9])?)/`),
Package: "mysql-cluster",
Package: mysqlClusterPackage,
PURL: mustPURL("pkg:generic/mysql-cluster@version"),
CPEs: []cpe.CPE{
cpe.Must("cpe:2.3:a:oracle:mysql:*:*:*:*:*:*:*:*", cpe.NVDDictionaryLookupSource),
@@ -412,7 +415,7 @@ func DefaultClassifiers() []binutils.Classifier {
FileGlob: "**/ndbd",
EvidenceMatcher: m.FileContentsVersionMatcher(
`/mysql-cluster-gpl-(?P<version>[0-9]+(\.[0-9]+)?(\.[0-9]+)?(alpha[0-9]|beta[0-9]|rc[0-9])?)/`),
Package: "mysql-cluster",
Package: mysqlClusterPackage,
PURL: mustPURL("pkg:generic/mysql-cluster@version"),
CPEs: []cpe.CPE{
cpe.Must("cpe:2.3:a:oracle:mysql_cluster:*:*:*:*:*:*:*:*", cpe.NVDDictionaryLookupSource),
@@ -423,7 +426,7 @@ func DefaultClassifiers() []binutils.Classifier {
FileGlob: "**/ndbmtd",
EvidenceMatcher: m.FileContentsVersionMatcher(
`/mysql-cluster-gpl-(?P<version>[0-9]+(\.[0-9]+)?(\.[0-9]+)?(alpha[0-9]|beta[0-9]|rc[0-9])?)/`),
Package: "mysql-cluster",
Package: mysqlClusterPackage,
PURL: mustPURL("pkg:generic/mysql-cluster@version"),
CPEs: []cpe.CPE{
cpe.Must("cpe:2.3:a:oracle:mysql_cluster:*:*:*:*:*:*:*:*", cpe.NVDDictionaryLookupSource),
@@ -434,7 +437,7 @@ func DefaultClassifiers() []binutils.Classifier {
FileGlob: "**/ndb_mgmd",
EvidenceMatcher: m.FileContentsVersionMatcher(
`/mysql-cluster-gpl-(?P<version>[0-9]+(\.[0-9]+)?(\.[0-9]+)?(alpha[0-9]|beta[0-9]|rc[0-9])?)/`),
Package: "mysql-cluster",
Package: mysqlClusterPackage,
PURL: mustPURL("pkg:generic/mysql-cluster@version"),
CPEs: []cpe.CPE{
cpe.Must("cpe:2.3:a:oracle:mysql_cluster:*:*:*:*:*:*:*:*", cpe.NVDDictionaryLookupSource),
@@ -16,6 +16,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/internal/testutils"
"github.com/anchore/syft/internal/unknown"
"github.com/anchore/syft/syft/file"
"github.com/anchore/syft/syft/pkg/cataloger/generic"
@@ -366,3 +368,49 @@ func Test_processDataTar_capsCopyrightFiles(t *testing.T) {
require.Error(t, err)
}
// Test_decompressionStream_failsEarlyInsteadOfBuffering pins that the bound is spent on read rather
// than after the fact. The existing bounds test uses a payload small enough that buffering the whole
// thing would pass anyway; this one is large enough that the difference shows up as bytes.
func Test_decompressionStream_failsEarlyInsteadOfBuffering(t *testing.T) {
const expanded = 512 * intFile.MB
// a few KB of input expanding to 512MB, so heap is what separates a streaming bound from a
// bound checked after the read completes
var gz bytes.Buffer
gw := gzip.NewWriter(&gz)
chunk := make([]byte, 32*1024)
for remaining := int64(expanded); remaining > 0; {
n, err := gw.Write(chunk[:min(remaining, int64(len(chunk)))])
require.NoError(t, err)
remaining -= int64(n)
}
require.NoError(t, gw.Close())
require.Less(t, gz.Len(), 1024*1024, "compressed payload should be tiny relative to what it expands to")
// both arms use io.ReadAll on purpose. The stream itself never holds much whatever the cap says,
// so copying to io.Discard measures nothing; what the cap actually protects is a consumer that
// buffers, and the real one (a tar reader building entries) does.
unbounded := testutils.MeasureAlloc(t, func() {
rc, err := decompressionStream(context.Background(), bytes.NewReader(gz.Bytes()), "control.tar.gz", expanded*2)
require.NoError(t, err)
defer rc.Close()
b, err := io.ReadAll(rc)
require.NoError(t, err)
require.Len(t, b, expanded)
})
require.Greater(t, unbounded, uint64(expanded),
"fixture did not actually cost more than its own size; it is no longer a bomb")
bounded := testutils.MeasureAlloc(t, func() {
rc, err := decompressionStream(context.Background(), bytes.NewReader(gz.Bytes()), "control.tar.gz", 64*1024)
require.NoError(t, err)
defer rc.Close()
_, err = io.ReadAll(rc)
require.ErrorIs(t, err, errDecompressedTooLarge)
})
t.Logf("unbounded allocated %d bytes, bounded allocated %d bytes", unbounded, bounded)
assert.Less(t, bounded, uint64(8*intFile.MB),
"a bounded stream must stop at the cap, not buffer the whole member and check afterwards")
}
@@ -7,13 +7,13 @@ import (
"debug/elf"
"encoding/binary"
"os"
"runtime"
"testing"
"github.com/kastenhq/goversion/version"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/anchore/syft/internal/testutils"
"github.com/anchore/syft/syft/internal/elfutil"
)
@@ -28,13 +28,13 @@ func Test_getBuildInfo_compressedSectionBomb(t *testing.T) {
t.Logf("%d byte fixture declares a %d byte section name table", len(bomb), declared)
// the unguarded path is the thing being defended against: prove the fixture really is a bomb
unguarded := measureAlloc(t, func() {
unguarded := testutils.MeasureAlloc(t, func() {
_, err := buildinfo.Read(bytes.NewReader(bomb))
t.Logf("buildinfo.Read err: %v", err)
})
assert.Greater(t, unguarded, uint64(declared), "fixture did not actually deliver the declared bytes")
guarded := measureAlloc(t, func() {
guarded := testutils.MeasureAlloc(t, func() {
_, err := getBuildInfo(bytes.NewReader(bomb))
require.Error(t, err)
assert.ErrorIs(t, err, elfutil.ErrDeclaredSizeExceeded)
@@ -53,13 +53,13 @@ func Test_getCryptoInformation_compressedSymtabBomb(t *testing.T) {
t.Logf("%d byte fixture declares a %d byte symbol table", len(bomb), declared)
// the unguarded path is the thing being defended against: prove the fixture really is a bomb
unguarded := measureAlloc(t, func() {
unguarded := testutils.MeasureAlloc(t, func() {
_, err := version.ReadExeFromReader(bytes.NewReader(bomb))
t.Logf("goversion err: %v", err)
})
assert.Greater(t, unguarded, uint64(declared), "fixture did not actually deliver the declared bytes")
guarded := measureAlloc(t, func() {
guarded := testutils.MeasureAlloc(t, func() {
_, err := getCryptoInformation(bytes.NewReader(bomb))
require.ErrorIs(t, err, elfutil.ErrDeclaredSizeExceeded)
})
@@ -149,18 +149,6 @@ func elfWithCompressedSymtab(t *testing.T, declared uint64) []byte {
return buf.Bytes()
}
// measureAlloc reports the bytes allocated while fn ran. TotalAlloc is process-wide, so a test using this
// must not call t.Parallel: another test's allocations would land in the measurement.
func measureAlloc(t *testing.T, fn func()) uint64 {
t.Helper()
var before, after runtime.MemStats
runtime.GC()
runtime.ReadMemStats(&before)
fn()
runtime.ReadMemStats(&after)
return after.TotalAlloc - before.TotalAlloc
}
// elfWithCompressedNameTable builds a minimal ELF64 whose only real section is a SHF_COMPRESSED .shstrtab
// declaring `declared` decompressed bytes and genuinely delivering them. Only the test that runs the
// unguarded path needs delivery; use elfDeclaringNameTable everywhere else, since building this one costs
@@ -15,6 +15,7 @@ import (
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/internal/testutils"
"github.com/anchore/syft/internal/tmpdir"
"github.com/anchore/syft/internal/unknown"
"github.com/anchore/syft/syft/file"
@@ -112,7 +113,7 @@ func TestDecompressUPX_SparsePlacementIsNotAnAllocationKnob(t *testing.T) {
require.ErrorIs(t, err, errUPXPartial)
require.NotNil(t, out)
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
_, _ = getBuildInfo(out)
})
@@ -17,6 +17,7 @@ import (
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/internal/spillbuf"
"github.com/anchore/syft/internal/testutils"
"github.com/anchore/syft/internal/tmpdir"
"github.com/anchore/syft/internal/unknown"
"github.com/anchore/syft/syft/file"
@@ -602,7 +603,7 @@ func TestDecompressUPX_RefusingATinyClaimIsCheap(t *testing.T) {
tiny := padTo(buildUPXFile(t, 1<<30, 1<<30,
[][]byte{bytes.Repeat([]byte("A"), 32)}, nil), 128)
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
_, err := unpackIn(t, t.TempDir(), tiny)
require.Error(t, err, "a 128 byte file may not claim a gigabyte")
})
@@ -629,7 +630,7 @@ func TestDecompressUPX_OutputIsNeverResident(t *testing.T) {
require.LessOrEqual(t, len(fixture), inputLen, "the fixture must stay small enough for the ratio to bind")
var out unpackedContents
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
var err error
out, err = unpackIn(t, t.TempDir(), fixture)
require.NoError(t, err)
@@ -702,7 +703,7 @@ func TestDecompressUPX_CompressedSizeCannotOutrunTheInput(t *testing.T) {
b[8] = 14 // b_method = LZMA
data := padTo(append(buildUPXHeader(4096, 4096), b...), 128)
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
_, err := unpackIn(t, t.TempDir(), data)
require.Error(t, err)
assert.ErrorIs(t, err, errUPXImplausibleHeader, "no block was readable, so nothing was packed")
@@ -842,7 +843,7 @@ func TestScanReader_MaliciousUPXRejected(t *testing.T) {
// assert on an already-finished test if it ever fired.
var build *extendedBuildInfo
var err error
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
build, err = scanReader(ctx, file.NewLocation("/malicious"), bytes.NewReader(data), false)
})
assert.Nil(t, build)
@@ -873,7 +874,7 @@ func TestCopyUnfiltered_WindowIsNotResident(t *testing.T) {
const size = 16 * intFile.MB
block := bytes.Repeat([]byte{0xE8, cto8, 0x11, 0x22, 0x33, 0x0F}, size/6+1)[:size]
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
require.NoError(t, copyUnfiltered(io.Discard, bytes.NewReader(block), size, cto8))
})
@@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/internal/testutils"
)
// readSeekCloser adapts a *bytes.Reader to the unionreader.UnionReader interface (adds Close).
@@ -122,7 +123,7 @@ func TestExtractDepsJSONFromELFBundle_MalformedFileszDoesNotOverAllocate(t *test
var content string
var err error
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
content, err = ExtractDepsJSONFromELFBundle(r)
})
require.NoError(t, err)
@@ -5,7 +5,6 @@ import (
"encoding/binary"
"io"
"math"
"runtime"
"testing"
"github.com/stretchr/testify/assert"
@@ -185,21 +184,6 @@ func (r *unbackedSizeReader) ReadAt(p []byte, off int64) (int, error) {
func (r *unbackedSizeReader) Close() error { return nil }
// measureAlloc reports the bytes allocated while fn runs. Only a byte count states "a small file cannot
// make us reserve a large buffer"; asserting an error comes back would keep passing if the allocation
// were hoisted above the check.
func measureAlloc(t *testing.T, fn func()) uint64 {
t.Helper()
var before, after runtime.MemStats
runtime.GC()
runtime.ReadMemStats(&before)
fn()
runtime.ReadMemStats(&after)
return after.TotalAlloc - before.TotalAlloc
}
func TestFindBundleHeaderOffset_ShortReadStillSearchesWhatWasRead(t *testing.T) {
data := fileWithSignatureAt(8192, 64, 0x1234)
@@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/internal/testutils"
)
func Test_extractDepsJSONFromBundle_Versions(t *testing.T) {
@@ -109,7 +110,7 @@ func TestExtractDepsJSONFromBundle_MalformedSectionSizesDoNotOverAllocate(t *tes
var content string
var err error
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
content, err = extractDepsJSONFromBundle(r, sections)
})
require.NoError(t, err)
@@ -4,7 +4,6 @@ import (
"bytes"
"encoding/binary"
"fmt"
"runtime"
"testing"
"time"
@@ -12,6 +11,7 @@ import (
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/internal/testutils"
)
const (
@@ -42,21 +42,6 @@ func boundWalk(data []byte) *resourceWalk {
return newResourceWalk(bytes.NewReader(data), testSectionRVA)
}
// measureAlloc reports the bytes allocated while fn runs. The property these guards exist for is "a small
// section cannot make us reserve a large buffer", and only a byte count states that: asserting an error
// comes back would keep passing if the allocation were hoisted above the check.
func measureAlloc(t *testing.T, fn func()) uint64 {
t.Helper()
var before, after runtime.MemStats
runtime.GC()
runtime.ReadMemStats(&before)
fn()
runtime.ReadMemStats(&after)
return after.TotalAlloc - before.TotalAlloc
}
func TestParseResourceDataEntry_SizePastSectionIsRejected(t *testing.T) {
// OffsetToData and Size are user-controlled uint32s. A data entry may only describe bytes its section
// actually holds, otherwise Size drives the allocation.
@@ -150,7 +135,7 @@ func TestParseResourceDirectory_AliasedEntriesCannotAmplifyWork(t *testing.T) {
var err error
var timedOut bool
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
done := make(chan error, 1)
go func() { done <- parseResourceDirectoryAt(testSectionRVA, w) }()
select {
@@ -360,7 +345,7 @@ func TestParseResourceDirectory_NestedOffsetsCannotUnderflow(t *testing.T) {
w := boundWalk(buf)
var err error
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
err = parseResourceDirectoryAt(testSectionRVA, w)
})
@@ -441,7 +426,7 @@ func TestProcessResourceEntry_HostileNameLengthIsRejected(t *testing.T) {
w := boundWalk(buf)
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
_ = processResourceEntry(entry, w)
})
@@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/internal/testutils"
)
func TestReadDataFromRVA_BogusSizeDoesNotOverAllocate(t *testing.T) {
@@ -21,7 +22,7 @@ func TestReadDataFromRVA_BogusSizeDoesNotOverAllocate(t *testing.T) {
r := &readSizeRecorder{Reader: bytes.NewReader(make([]byte, fileSize))}
var err error
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
_, err = readDataFromRVA(r, 0x1000, 0xFFFFFFFF, sections)
})
@@ -73,7 +74,7 @@ func TestReadDataFromRVA_SizePastTheAbsoluteCapIsRejected(t *testing.T) {
r := &readSizeRecorder{Reader: bytes.NewReader(make([]byte, size+1))}
var err error
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
_, err = readDataFromRVA(r, 0x1000, size, sections)
})
@@ -334,7 +334,7 @@ func getMetadataTypeName(metadata any) string {
}
// handle pointers
if t.Kind() == reflect.Ptr {
if t.Kind() == reflect.Pointer {
t = t.Elem()
}
@@ -407,7 +407,7 @@ func dereferenceToStruct(v any) reflect.Value {
}
val := reflect.ValueOf(v)
if val.Kind() == reflect.Ptr {
if val.Kind() == reflect.Pointer {
if val.IsNil() {
return reflect.Value{}
}
@@ -436,7 +436,7 @@ func hasPopulatedDigest(fileRecord reflect.Value) bool {
// check if digest is a pointer and not nil, or a non-zero value
switch digestField.Kind() {
case reflect.Ptr:
case reflect.Pointer:
return !digestField.IsNil()
case reflect.String:
return digestField.String() != ""
@@ -10,13 +10,13 @@ import (
"io"
"os"
"path"
"runtime"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/internal/testutils"
"github.com/anchore/syft/syft/artifact"
"github.com/anchore/syft/syft/cpe"
"github.com/anchore/syft/syft/internal/unionreader"
@@ -299,20 +299,6 @@ func buildMinimalPE64(exportRVA, exportSize uint32, totalSize int) []byte {
return buf
}
// measureAlloc reports the bytes allocated while fn runs. TotalAlloc is process-wide, so a test using
// this must not call t.Parallel.
func measureAlloc(t *testing.T, fn func()) uint64 {
t.Helper()
var before, after runtime.MemStats
runtime.GC()
runtime.ReadMemStats(&before)
fn()
runtime.ReadMemStats(&after)
return after.TotalAlloc - before.TotalAlloc
}
// readSizeRecorder records the largest single read requested of it. A small allocation is not enough on
// its own; no single read may exceed the file size either, whatever the headers claim.
type readSizeRecorder struct {
@@ -342,7 +328,7 @@ func TestNewPE_ExportDirectorySizeIsNotAllocatedUpFront(t *testing.T) {
r := &readSizeRecorder{Reader: bytes.NewReader(data)}
var err error
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
_, err = newPE("oversized-export-dir.exe", r)
})
@@ -501,7 +487,7 @@ func TestDecompressSbom_RejectsStreamExpandingPastTheLimit(t *testing.T) {
binary.LittleEndian.PutUint64(dataBuf[lengthStart:], lengthStart)
var decompressErr error
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
_, _, decompressErr = decompressSbom(dataBuf, 0, lengthStart)
})
err = decompressErr
@@ -14,6 +14,7 @@ import (
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/internal/testutils"
"github.com/anchore/syft/syft/artifact"
"github.com/anchore/syft/syft/pkg"
)
@@ -344,7 +345,7 @@ func TestReadExportDirectory_DoesNotReadThePhantomBytes(t *testing.T) {
rec := &readSizeRecorder{Reader: bytes.NewReader(make([]byte, realSize))}
dir := pe.DataDirectory{VirtualAddress: 0, Size: nativeImageMaxExportDirectorySize}
allocated := measureAlloc(t, func() {
allocated := testutils.MeasureAlloc(t, func() {
_, err := readExportDirectory(rec, dir)
// a Size exactly at the cap must fall through to the remaining-bytes check, not be capped away;
// pins the `>` (not `>=`) in the cap comparison
@@ -205,7 +205,7 @@ func (r *Resolver) resolveProjectProperty(ctx context.Context, resolutionContext
pomValue = pomValue.Field(fieldNum)
pomValueType = pomValue.Type()
if pomValueType.Kind() == reflect.Ptr {
if pomValueType.Kind() == reflect.Pointer {
// we were recursing down the nested structs, but one of the steps
// we need to take is a nil pointer, so give up
if pomValue.IsNil() {
@@ -16,6 +16,9 @@ import (
"github.com/anchore/syft/syft/pkg/cataloger/internal/binutils"
)
// phpCPEName is the vendor and product used for every PHP interpreter CPE below.
const phpCPEName = "php"
type interpreterCataloger struct {
name string
extensionsGlob string
@@ -49,8 +52,8 @@ func NewInterpreterCataloger() pkg.Cataloger { //nolint:funlen
{
Attributes: cpe.Attributes{
Part: "a",
Vendor: "php",
Product: "php",
Vendor: phpCPEName,
Product: phpCPEName,
},
Source: cpe.NVDDictionaryLookupSource,
},
@@ -71,8 +74,8 @@ func NewInterpreterCataloger() pkg.Cataloger { //nolint:funlen
{
Attributes: cpe.Attributes{
Part: "a",
Vendor: "php",
Product: "php",
Vendor: phpCPEName,
Product: phpCPEName,
},
Source: cpe.NVDDictionaryLookupSource,
},
@@ -86,15 +89,15 @@ func NewInterpreterCataloger() pkg.Cataloger { //nolint:funlen
Package: "libphp",
PURL: packageurl.PackageURL{
Type: packageurl.TypeGeneric,
Name: "php",
Name: phpCPEName,
// the version will be filled in dynamically
},
CPEs: []cpe.CPE{
{
Attributes: cpe.Attributes{
Part: "a",
Vendor: "php",
Product: "php",
Vendor: phpCPEName,
Product: phpCPEName,
},
Source: cpe.NVDDictionaryLookupSource,
},
@@ -11,6 +11,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/anchore/syft/internal/testutils"
"github.com/anchore/syft/syft/file"
"github.com/anchore/syft/syft/pkg"
"github.com/anchore/syft/syft/pkg/cataloger/generic"
@@ -310,3 +311,47 @@ func TestParseKernelChangelog(t *testing.T) {
})
}
}
// TestParseKernelChangelog_gzipBombStaysBounded pins a bound nothing in this file states out loud.
// parseKernelChangelog reads the decompressed changelog through a bufio.Scanner, and Scanner refuses a
// token past MaxScanTokenSize, so a gzip member expanding to gigabytes costs 64KB. That is the only
// thing holding this path, and it is incidental: switching to io.ReadAll or a bufio.Reader to pick up
// long lines would remove it silently, which is what this test is here to catch.
func TestParseKernelChangelog_gzipBombStaysBounded(t *testing.T) {
const expanded = 512 * 1024 * 1024
// one enormous line with no newline, the worst case for a line scanner
var gz bytes.Buffer
gw := gzip.NewWriter(&gz)
chunk := bytes.Repeat([]byte{'a'}, 32*1024)
for remaining := expanded; remaining > 0; {
n, err := gw.Write(chunk[:min(remaining, len(chunk))])
require.NoError(t, err)
remaining -= n
}
require.NoError(t, gw.Close())
require.Less(t, gz.Len(), 1024*1024, "compressed payload should be tiny relative to what it expands to")
// prove the fixture is a bomb: the same stream read without a line bound costs its full size
unbounded := testutils.MeasureAlloc(t, func() {
zr, err := gzip.NewReader(bytes.NewReader(gz.Bytes()))
require.NoError(t, err)
defer zr.Close()
b, err := io.ReadAll(zr)
require.NoError(t, err)
require.Len(t, b, expanded)
})
require.Greater(t, unbounded, uint64(expanded),
"fixture did not actually cost more than its own size; it is no longer a bomb")
bounded := testutils.MeasureAlloc(t, func() {
_, _, err := parseKernelChangelog(context.Background(), nil, nil,
file.NewLocationReadCloser(file.NewLocation("/changelog.gz"), io.NopCloser(bytes.NewReader(gz.Bytes()))))
// the scanner refuses the oversized line, so this fails rather than parsing a version out of it
require.Error(t, err)
})
t.Logf("unbounded allocated %d bytes, bounded allocated %d bytes", unbounded, bounded)
assert.Less(t, bounded, uint64(8*1024*1024),
"the changelog scan must stay bounded by the line limit, not by what the member expands to")
}
+35
View File
@@ -38,6 +38,41 @@ func noUnboundedReads(m dsl.Matcher) {
Report("do not use unbounded io.Copy to in-memory buffer; wrap the source reader with io.LimitReader")
}
// nolint:unused
func noOverflowingBoundsCheck(m dsl.Matcher) {
// an offset+size bounds check on unsigned operands wraps rather than saturating, so a large
// offset out of a file header can produce a small sum and pass a check it should have failed.
// Written as a subtraction against the limit, there is nothing to wrap.
m.Match(
`$offset + $size > $limit`,
`$offset + $size >= $limit`,
`$offset + $size < $limit`,
`$offset + $size <= $limit`,
).
Where((m["offset"].Type.OfKind("unsigned") || m["size"].Type.OfKind("unsigned")) &&
// a constant addend cannot be attacker-controlled, which is the common safe shape of
// stepping a loop cursor (`pos+4 <= uint32(len(buf))`)
!m["offset"].Const && !m["size"].Const &&
// same 32-bit floor as noUnboundedAllocations: a uint16 sum is a nibble or a count widened for
// the comparison, not an offset that can carry a file-controlled value up to the wrap point
m["offset"].Type.Size >= 4).
Report("bounds check can overflow: $offset + $size wraps on unsigned operands. write it as a subtraction against the limit instead, e.g. `$offset > $limit || $size > $limit-$offset`")
}
// nolint:unused
func noSwallowedEOF(m dsl.Matcher) {
// treating EOF as success hands the caller a zero value it cannot distinguish from real data. In a
// loop that advances by however much was read, that is not just bad data but a cursor that never
// moves, so a truncated record spins forever.
m.Match(
`if errors.Is($err, io.EOF) { return nil }`,
`if errors.Is($err, io.EOF) { return nil, nil }`,
`if $err == io.EOF { return nil }`,
`if $err == io.EOF { return nil, nil }`,
).
Report("EOF returned as success: the caller gets a zero value it cannot tell from real data. return the error, or return an explicit sentinel the caller checks")
}
// nolint:unused
func noDirectTempFiles(m dsl.Matcher) {
// catalogers must use tmpdir.FromContext(ctx) instead of creating temp files/dirs directly,