* 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>
`.ko.gz`/`.ko.xz`/`.ko.zst` modules are spilled to a temp file so ELF parsing
can seek over them. The compressed size is bounded by the file in the image but
what it decompresses to is not, so a few KB of crafted zstd filled the temp
directory until the disk did.
`io.Copy` now stops one byte past a 256MB cap and rejects rather than
truncating, since a partial ELF reads as a corrupt module instead of a rejected
one. The partial spill is removed on the reject path.
The cap is a parameter on an inner function so the tests exercise the boundary
with a few hundred bytes instead of materializing 256MB, matching
`decompressSbomWithLimit` in the graalvm cataloger.
The cap bounds one spill, not the process: peak disk is 256MB times however many
modules are parsed at once, which is cataloger parallelism (`NumCPU*4` by
default, and unbounded when `--parallelism` is negative).
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(arch): bound the decompressed mtree listing
The ALPM cataloger handed a gzip stream straight to `mtree.ParseSpec`, which
materializes every entry before it returns any of them. Nothing capped the
decompressed size, so a crafted mtree in a scanned image expanded until the
process died. 64KB of input is enough to produce 64MB, and the ratio scales.
Reads through a limit now and rejects a listing that exceeds it. The read goes
one byte past the cap so that tripping it is distinguishable from a listing that
simply ends there, since silently truncating would produce a package missing
most of its files. The existing error path already surfaces this as an unknown.
The cap is well above any real package: an mtree names every file with its
digests, and even the largest packages land far under it.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(arch): bound mtree entry count, not just decompressed bytes
The byte cap alone does not bound what the parse retains. go-mtree keeps an
entry per line and appends it unconditionally, blank lines included, so short
lines cost about 1KB of heap each.
Measured on the previous commit: 15KB of gzipped newlines expanding to 16MB,
comfortably inside the 64MB byte cap, drove 8.1GB of peak heap over 74 seconds
and returned no records and a nil error. Same input now rejects in 0.05s at
51MB peak.
Counting lines before parsing is what makes the entry allocation bounded, since
the parser materializes all of them before returning. Lines the parser skips
only make the count high, never low.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(arch): apply the mtree bounds while streaming
Both caps were enforced against a fully buffered listing, so the peak cost
scaled with cataloger parallelism, which runs a goroutine per package. They now
apply as the stream flows: a limited reader for the byte cap and a counting
reader for the entry cap, which has to trip while the parser is still reading
since it materializes every entry before returning any of them.
Both limits are injectable so the boundary tests can be exact on both sides
without allocating their way up to the shipped 64MB. Each cap gets a listing
exactly at it, asserting the records parse whole rather than just that no error
came back, since quiet truncation is the failure a size check invites.
The byte limiter carries one byte of headroom, which is what makes an
overrunning listing distinguishable from one that simply ends at the cap.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(arch): refuse mtree line continuations
go-mtree collapses a line ending in a backslash into the next one with a
quadratic string concat, so the cost is lines times bytes and neither the byte
cap nor the line cap can see it. 199k continued lines sitting inside both caps
measured 4m34s and 6.1TB of allocation churn, and returned no error.
Also re-derived both constants against a real package rather than a guess.
`texlive-fontsextra` is 105,847 files and 1,978 directories, so roughly 108k
lines at 150-250 bytes each. 16MB would have falsely rejected it, so the byte
cap stays at 64MB and the line cap goes 200k -> 300k, which is ~2.8x a real
package instead of 1.85x.
Rejections are now sentinels so tests can name which bound tripped, and the
constants carry the measured ceiling they imply: a crafted listing of minimal
fields still reaches ~1.1GB at the byte cap, since go-mtree allocates per
whitespace-delimited field. Bounding that further means bounding fields rather
than bytes, and legitimate input is already within ~1.3x of the crafted worst
case, so there is little to win.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(arch): refuse a continuation spelled with a carriage return
The refusal matched the literal bytes backslash-newline, but go-mtree decides
"this is a continuation" from the scanner token, and `bufio.ScanLines` drops a
trailing carriage return before testing for the backslash suffix. So a listing
using CRLF ran the quadratic collapse with nothing watching.
Measured: 775KB of gzip expanding to 17MB across 150k lines, inside both other
bounds, took 1m33s and returned a nil error. Same input now rejects in 50ms.
The check is a single byte pass that counts newlines and looks for a backslash
before either line ending, which replaces the two scans and the straddle special
case it took to catch only one spelling. A tripped bound is also latched now:
`bufio.Scanner` stops at the first error, but nothing about the type said a
caller had to, and the refusal was recomputed per chunk.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* test(arch): make the bombpkg fixture able to fail
The fixture was 310k bare newlines, which go-mtree turns into blank entries with
no name, and the parse loop already skips those. So the assertion that a rejected
listing leaves an empty file list held whether or not the bound existed. It is now
a repeated file line: 0 records with the bound, 310,000 without.
Dropped the two tests that asserted nothing as a result. The cataloger-level one
was a weaker restatement of what the golden comparison already pins exactly, and
the shipped-size-cap one drove a real 64MB listing through the parser to prove a
constant had not moved. Asserting the constants directly catches any change rather
than only a loosening.
Race-mode package runtime goes 15.7s -> 4.4s.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* test(arch): make three mtree assertions able to fail
The truncated-gzip case asserted the failure was not misreported as the size
bound, but ran against the shipped 64MB cap on a 3.5KB listing, so the check it
guarded could never fire. It now runs against a cap just above the truncation
point, where the two genuinely race.
The bomb case asserted an allocation delta from `runtime.MemStats`, which is a
process-wide cumulative counter and measures the property indirectly. It now
counts bytes pulled through the reader, which is what "gave up while reading"
actually means and does not drift with GC or parallelism.
Added the one bound that had no test at all. A single line over 64KB is refused
by go-mtree's scanner, not by anything here, so a dependency bump that called
`scanner.Buffer` would remove it silently.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* docs(arch): correct the mtree bound comments and close the gzip reader
The comment said both bounds apply while the listing streams. Only the line cap
does. `io.LimitedReader` signals a drained budget with `io.EOF`, which the parser
reads as the end of a complete listing, so the byte cap is necessarily a check
after the fact. Peak cost is the same either way, since accepting a listing at
the cap costs what rejecting one just past it does, but the comment was claiming
a mechanism that isn't there.
Also trimmed the benchmark writeups down to the reasoning that outlives them.
The exact transient-heap and wall-clock figures are tied to a go-mtree version
with nothing to detect them going stale, and one of them described the CRLF case
as prevented when it wasn't. And `TestMtreeParse` was labelled the regression
guard for the continuation refusal, but its fixture has no backslashes in it.
`gzReader` now gets a `defer Close` to match the other gzip site in the tree.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(arch): stop the alpm database parser panicking on a malformed backup line
A `%BACKUP%` entry is `path\tmd5`, and the parser indexed the second field
without checking the split produced one. A line with no tab panics. The database
is attacker-controlled, and while the task executor recovers, it recovers at task
granularity, so one crafted line drops every Arch package from the SBOM.
The same loop also never checked the scanner's error. A field over the 1MB token
cap ends the scan, and the entry came back populated up to that field and empty
after it, with a nil error saying it was fine. That is the failure the mtree
bounds in this branch exist to avoid, two functions up in the same file.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* docs(arch): trim the comments added by this branch
the comments were carrying reasoning that belongs in the PR description rather
than the code. Kept the parts a future reader cannot recover from the code (the
cap sizing, why continuations are refused, why the byte check runs after the
parse) and cut the rest.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* test(arch): generate the mtree bomb fixture instead of committing it
the fixture is a synthetic listing fully described by a line count and a
repeated line, but it landed as a committed gzip member, which shows neither and
leaves no way to rebuild it when maxMtreeLines moves. the two tests that walk
testdata/installed write it first now, and git ignores it.
the real pacman listings (gmp, tree-sitter) stay committed since those are
captured output rather than something a rule can reproduce.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* correct and simplify comments
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(java): bound user-controlled reads in the graalvm native-image cataloger
Sizes, offsets and lengths in a native-image binary all come from the file
being parsed, and several were acted on directly.
The PE export directory sized a `make([]byte, Size)` from a uint32 in the
optional header, so an 8KB file claiming 4GB reserved 4GB before reading a
byte. It now reads what the file actually holds, bounded by the declared size,
and still requires the whole directory to be present. This also fixes a latent
bug: `unionreader.readerAtAdapter.ReadAt` can return a short read with a nil
error, and the old code discarded the count, so squashfs-sourced binaries could
parse zero padding as export data.
The bounds checks in `decompressSbom` and the PE export walk validated only the
end of a range, computed by adding to a value out of the file. Those sums wrap,
and a wrapped sum compares as in range while the slice that follows it panics.
They now subtract from the known-good length instead. The three
`address - sectionBase` subtractions are unsigned and underflowed on an address
below the base; they share one guarded helper now.
The embedded SBOM decompresses through an `io.LimitedReader`, since the
compressed bytes are bounded by the file but what they expand to is not. The
limit is checked before the decoder's own error, which would otherwise report a
size problem as "not a cyclonedx json document". Hitting it is logged, since
the caller reports parse failures at trace level and a dropped SBOM would
otherwise go unnoticed.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(java): stop at the first format that parses a native image
The format loop tried ELF, Mach-O and PE against every reader and kept going
after one of them parsed, so a file that parses as more than one format
contributes its packages once per format.
No input parses as two formats today, so this is not a behavior change in
practice: `debug/pe` rejects an ELF on the optional-header magic, and both
`debug/elf` and `debug/macho` reject anything without their own magic. It
removes the possibility rather than a live bug.
Note this does not deduplicate across readers, which is the outer loop: a
universal Mach-O carrying the same native image for two architectures still
yields its packages twice.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(java): bound the graalvm export directory read against the file
Follow-up to the hardening earlier in this branch, from review.
The export directory read was still bounded only by file size, so a large
mostly-empty file (cheap to ship inside a compressed layer) was read into
memory in full and then discarded. It now weighs the declared size against an
absolute cap and against the bytes really present before allocating anything,
matching `readDataFromRVA` in `syft/pkg/cataloger/internal/dotnet/pe`. A 64MB
file declaring a 4GB export directory went from ~140MB allocated to ~3.6KB.
Also from the same review:
- dropped the decompressed-SBOM ceiling from 100MB to 16MB. Decoding costs
roughly 48x the JSON size in allocation and 6x retained, so the old ceiling
was worth several GB of churn per file, and catalogers run concurrently.
- `symbolOffsets` is now single-address `symbolOffset`. Two same-typed address
arguments in a row could be transposed at a call site and would produce a
silently wrong SBOM rather than an error.
- `newMachO` now returns an error on a real read failure. It was falling
through and dropping the file with no error and no log at all.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(java): reject a short read of the graalvm export directory
Second round of review on this branch.
The export directory read introduced earlier in the branch only consulted the
byte count when `ReadAt` also returned an error, so a reader handing back fewer
bytes with a nil error was accepted and the tail of the buffer stayed zeroed,
then got parsed as a valid export directory. The count decides now, whatever
the error says. `io.ReadFull` would also cover it but spins forever on a reader
that keeps answering `(0, nil)`, which is the same hostile input this is meant
to reject, so the single read stays.
Also in here:
- the decompressed SBOM is buffered and size-checked before the decoder is
invoked, instead of inferring the overrun from `io.LimitedReader.N` after the
fact. That check was only correct because the cyclonedx decoder happens to
drain its reader before parsing, and nothing would have caught it if that
changed.
- four bounds guards, the `newMachO` error path and the SBOM ceiling had no
test that failed when they were removed. They do now, verified by deleting
each one and watching a named test go red. The ceiling in particular could be
raised to 2GB with the suite still green, since every case derived its sizes
from the constant.
- `nativeImagePE.reader` is dead now that the read takes the reader as an
argument.
- three comments claimed guarantees the code does not make: that `symbolOffset`
cannot be called with transposed arguments, that a rejected file cannot go
unrecorded when it is only logged at trace level, and a set of measured
allocation multipliers that nothing pins.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* chore(java): trim the graalvm cataloger comments
Comment pass over everything this branch added. Cut about a quarter of the
comment lines, mostly restatement and detail that was never going to stay true.
Dropped: a production comment naming a test, the measured allocation multiples
behind the two size ceilings, cross-references to the equivalent constants in
`dotnet/pe` and `elfutil`, and test docs that restated their own test name
before saying anything.
Kept the parts that fail silently if someone tidies them away: why the read is
a single `ReadAt` rather than `io.ReadFull`, the note that `readExportDirectory`
uses an RVA directly as a file offset, and the fixture notes explaining why a
`VirtualAddress` is deliberately huge or why no symbol sits at index 0.
Three that were stale rather than verbose:
- `nativeImagePE`'s doc still said it holds the underlying reader, which this
branch deleted.
- `TestFetchSbomSymbols_FindsAllThreeSymbols`' doc had ended up above
`TestFetchSbomSymbols_AddressBaseOutOfRangeGuard`, documenting the wrong
function.
- a comment referenced a commit sha that no longer exists on this branch.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(dotnet): bound the bundle signature search by the file, not the headers
The single-file bundle marker search sized its buffer from `calculatePEEndOffset` /
`calculateELFEndOffset`, both of which sum user-controlled header fields. A tiny file
declaring 4GB sections drove an 8.6GB allocation, and on the ELF side the sum could
overflow negative and panic in `make`.
The search now clamps its window to the real file length before sizing anything. The PE
and ELF paths had grown two copies of the same find-and-read sequence, so they collapse
into one `bundle.ExtractDepsJSON`, which lets the signature search and the header reader
both go unexported.
Also guards `read7BitEncodedInt` against returning a negative length, which on a 32-bit
`int` would seek a manifest walk backwards and re-read the same bytes per declared file.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(dotnet): stop a truncated version resource from spinning forever
`readIntoStruct` reported a truncated read as success and left the destination struct
zeroed. Its callers advance their loop counters by the offsets it updates, so the string
table loop consumed nothing, advanced nothing, and never reached its length bound: a
version resource whose header claims more content than the blob carries hung the parse.
A truncated read is now an error, and the two places where hitting the end of a blob is
normal say so explicitly. That distinction is what keeps a well-formed resource whose last
child is `VarFileInfo` from losing its `FileVersion` fallback. io.EOF and
io.ErrUnexpectedEOF mean the same thing here, so both take the same path.
The string table walk moves into its own function, since the two new arms put
`parseVersionResourceSection` over the funlen limit.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(dotnet): bound PE resource parsing by what the file actually holds
Resource sizes and RVAs are user-controlled uint32s that sized buffers before anything
checked the file held that many bytes, so a small crafted section could reserve up to 4GB.
The walk now carries the section's reader and its base RVA for its whole lifetime, so
`reader.Size()` is the single authoritative bound and one `offsetOf` helper rejects every
RVA the section does not hold. That replaces the per-node section value the recursion used
to synthesize, whose size argument was only correct by convention.
Bounding the allocations alone turns the OOM into a hang, though: nothing in the format
stops entries from aliasing, so thousands of them may name one fat blob and every offset
still checks out. A 256KB section built that way drove 24GB of allocation over two minutes
with peak memory flat. Capping entry *count* does not help since the cost is per byte read,
so the walk now charges every read against a budget of a few times the section size. Real
binaries come in at about 1x, since a well-formed section's entries partition it.
A resource tree that stops early no longer fails the whole file. It says nothing about the
CLR directory or an embedded deps.json, and dropping the package from the SBOM over one
malformed section is worse than reporting the fields we did get.
Also widens the entry counts before summing them (two uint16s can sum past 0xFFFF and wrap),
and bounds the name length and section header count against the reader for the same reason.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(dotnet): bound resource recursion depth and cap declared sizes absolutely
Review follow-ups on this branch. The headline one is a real crash: the resource
walk had no depth bound, and the `budget` comment claiming depth "falls out for
free" was wrong, since the budget scales with the section while the stack does
not. A chain of directories at distinct RVAs (which the `dirs` set does not
catch) recursed until the goroutine stack gave out. A 13.7MB crafted PE took the
whole process down with `fatal error: stack overflow`, which is unrecoverable,
so unlike every other bug on this branch no caller could log-and-continue past
it. Now capped at 32 levels, versus the 3 the format actually uses.
The other bounds moved from "the file" to a real ceiling. Clamping an allocation
to the bytes remaining in the file is not enough on its own: a mostly empty file
costs almost nothing inside a compressed layer, so a cheap artifact could still
authorize a multi-gigabyte read. Both the PE data directories and the bundle
marker search now cap the same way `elfutil` already does.
Also hardened in the bundle reader:
- `headerOffset` is validated against the file. It comes straight out of the
file bytes and everything downstream seeks to it, so it was the least
trustworthy value in there and the only one not being checked.
- a manifest entry count that could not fit in the remaining bytes is rejected
rather than walked for hundreds of millions of reads.
- a non-positive `searchLimit` falls back to the whole file instead of reading
nothing. It is only ever an optimization hint, so treating it as authoritative
let one bogus ELF `p_offset` hide a bundle from us entirely.
- a short read no longer aborts the search. `unionreader` can return fewer bytes
than the file reports, and the marker may well be in what we did get.
`pe.Read` now reports partial parses instead of choosing between dropping the
package and hiding the problem. It was inconsistent: a malformed resource tree
was non-fatal while an unreadable resource *section* one call away was fatal, and
the non-fatal path only reached a trace log, so the unknowns task could not see
that a file had been cataloged on incomplete evidence. Both now land on a new
`File.ParseErr`, which the dotnet and PE catalogers join into their unknowns and
still keep the package.
Worth noting for changelog purposes: the `isTruncated` guard fixes a
user-visible bug beyond the hardening. A version resource leaving 1 to 5
trailing bytes cuts the next struct in half, which reports as
`io.ErrUnexpectedEOF` rather than `io.EOF`; that used to propagate out and skip
the `VS_FIXEDFILEINFO` fallback, so those binaries lost their version entirely.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(dotnet): treat a zero bundle offset as unbundled, not malformed
The apphost ships the 32-byte bundle signature compiled in with its 8-byte header
offset left as zero, and only `HostWriter.SetAsBundle` fills that slot in when you
publish as a single file. So a zero offset means "never bundled", which is what
every framework-dependent .NET executable looks like.
The bounds work on this branch folded that case in with the hostile ones, so the
most common shape a .NET binary takes started reporting a parse failure. It
surfaces through the PE cataloger rather than the dotnet one, since an apphost
carries no CLR metadata of its own: the package still comes out, with a spurious
unknown attached to it. `main` had this right and said so in a comment; the
regression is on this branch.
Reproduces on a fixture already in the repo. Of the 53 PE files in
`image-net8-app`, `/app/dotnetapp.exe` was the only one reporting anything, and
the whole image is clean again now.
The three outcomes are now distinct, since collapsing any two of them loses
something the caller needs: zero means there is no bundle, an error means we could
not tell, and a positive offset is the answer.
`Test_Read_DotNetDetection` gains the apphost and, more usefully, an assertion
that a well-formed binary reports no partial parse at all. That one line is what
would have caught this, and it now covers all six fixtures. The unit case that
pinned the old behavior asserted the bug, so it flips.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(dotnet): measure readers with the shared helper and report refusals
Two things, both about not re-deriving what the campaign already has.
`internal/file.ReaderSize` exists on `main` for exactly this job, and `upx.go`
already uses it. Its doc comment is the problem statement here almost verbatim: a
declared size is worth nothing until you weigh it against a byte count the reader
can actually back, which it does by reading the last byte. This branch was
hand-rolling `Seek(0, io.SeekEnd)` in three places, one of them reimplementing the
helper's save-and-restore dance verbatim, and then absorbing the reader-lies case
downstream by tolerating short reads.
Adopting it means the bundle reader and `readDataFromRVA` now take the reader
types they were always being handed (`unionreader.UnionReader` and `io.ReaderAt`),
and the reads move to `ReadAt`, which leaves the caller's cursor alone and drops
two seeks. Net effect is a smaller diff than what it replaces, and the case the
`shortReader` fake was invented for is now caught up front instead of tolerated:
the fake was modeling a lie about length, which is not what a short squashfs read
actually does. It now models the real shape (honest length, read stops early) and
a second reader covers the lie.
Three places also still returned "there is nothing here" when what they meant was
"I could not tell", which is the thing `File.ParseErr` was added to stop:
- an ELF that `elfutil` refused to parse read as "not an ELF". `ErrDeclaredSizeExceeded`
is exported precisely so a refusal can be told apart from a broken file, and the
caller upstream already turns an error into an unknown.
- a `deps.json` past the 3MB cap returned "". That is the file's entire dependency
list, so this handed back an SBOM that looked complete and silently was not.
- the marker search clamped at 512MB and then reported no bundle. It cannot claim
there is no bundle when it declined to look everywhere one could be, so a miss
after clamping is now an error.
`readSizeRecorder` records `ReadAt` alongside `Read`, or the allocation-bound tests
would have quietly stopped measuring anything.
Also renames `findSignatureOffset` back to `findBundleHeaderOffset`, since it
returns a header offset and reading it as "no signature" is what the last bug was,
and fixes the stale `ExtractDepsJSONFromBundle` doc comment.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* test(dotnet): assert allocation sizes directly in the bundle search guards
The PE and ELF search guards asserted the largest read the reader was asked for.
That happened to track the allocation, since the whole buffer goes to ReadAt, but
nothing said so. Both now measure the bytes allocated with the same `measureAlloc`
helper the resource guards use, and keep the read-size check as a separate claim.
Also names the reason a reader can report a size it cannot back: `debug/elf` and
`debug/pe` hand out `io.NewSectionReader(r, 0, 1<<63-1)`, whose `Size()` is nominal
rather than measured, so answering a size is not on its own evidence the bytes are
there. Renames `lyingReader` to `unbackedSizeReader` and regroups the two reader
implementations, which had their methods interleaved.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* test(dotnet): speed up pe fixture tests
the pe package took ~90s under -race, nearly all of it loading docker archive
fixtures. two things:
the three bundle fixtures published with `SelfContained=true`, so each `hello.exe`
carried a full .NET runtime (~60MB, 33MB tar). the bundle format is what those
fixtures exercise and it is identical framework-dependent, so they now publish
with `SelfContained=false` (160KB exe, 2MB tar). the race detector multiplies the
layer inflate + diffID hashing by ~15x, which is why the size mattered so much.
`fixtureFile` also loaded the image and built a squashed resolver per table row,
so `image-net8-app` was loaded four times for four files in it. resolvers are now
built once per distinct fixture from the parent test.
87s -> 28.5s under -race, and ~60MB less fixture cache.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(dotnet): read the right file type from V1 single-file bundles
V1 bundles (.NET Core 3.x) predate the `unknown` member at the head of the
manifest file type enum, so their types are all one less than V2+: deps.json is
2, runtimeconfig.json is 3. we were matching on the V2+ value, which silently
matched runtimeconfig.json instead, so every .NET Core 3.x single-file app
reported its runtimeconfig as the embedded deps.json and its dependencies went
unseen.
the existing fixture test asserted the buggy output (it expected
`runtimeOptions`, which only ever appears in a runtimeconfig). the assertions
now pin the framework moniker out of each fixture's real deps.json, so reading
the neighboring file can't pass.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* test(dotnet): drop the 64MB jruby binary from the net8-app fixture
the `image-net8-app` fixture wget'd a 64MB jruby release binary at build time to
serve as "a PE that is not a .NET binary", which was 64MB of the image's 65MB.
every test that loads that image pays for it, in both the dotnet cataloger and
the pe package, and it sits in the fixture cache twice.
the same jruby binary already has its own fixture in the binary cataloger
(`image-jruby`), asserted through the same `pe.Read`, so the coverage is not
lost. the negative case here now uses `/app/dotnetapp.exe` -- the native apphost
that is already in the image -- which pairs nicely with the single file case,
same apphost with the app bundled in and CLR evidence present.
also drops a github release download from the fixture build.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix oom when testing
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* right-size CI test runners
the `r` family and `ram=32+128` were sized for the tmpfs footprint rather
than the workload. with the caches back on disk the build+test peak fits in
16GB, so move to `m`/`c` at `ram=16+32`.
- ~27% cheaper per instance-hour on spot, and a much deeper pool to schedule
against (~20 x86 types vs 7)
- `cpu=4+8` with `ram=16+32` lets price-capacity-optimized opportunistically
land an 8 vCPU box when it's cheap
- plain `volume=100gb` replaces the provisioned throughput. a 4 vCPU instance
caps near 150MB/s of EBS and gp3's free baseline is already 125MB/s, so the
override bought nothing
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
the base image is digest-pinned to a bullseye snapshot whose InRelease has
expired, so `apt-get update` now exits 100 on any cold fixture build. rather
than disable Check-Valid-Until (which just delays the next break), skip apt
entirely:
- `less` was never used and `unzip` already ships in the base image
- rezip with `jar cfM` from the base JDK instead of installing `zip`
the `-M` matters here: this fixture intentionally has no META-INF/MANIFEST.MF,
and plain `jar cf` would generate one.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
`archive.ubuntu.com/ubuntu/pool/main` only keeps the current version of a
package, so the pinned `zlib1g 1:1.3.dfsg-3.1ubuntu2.1` URL started 404ing.
`curl` without `-f` wrote the error page into `zlib1g.deb` and exited 0, so
the fixture image built clean and just contained a non-deb, failing
`TestDpkgArchiveCataloger` only when the fixture cache was cold.
Now points at the launchpad `+files` URL, which is immutable. Also added `-f`
to the other fixture Dockerfiles that curl a pinned artifact so the next bit
of URL rot fails the build instead of quietly poisoning a fixture.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(golang): bound UPX decompression by what the input can justify
A UPX block's `sz_unc` was passed straight to `make([]byte, n)`, so four bytes of
attacker input spanned the full uint32 range. A ~300KB file could drive multi-GB
resident memory, which Go reports as a fatal runtime OOM that no `recover` on this
path can contain. Syft parses binaries out of arbitrary images, so that is reachable
from any scan.
Two bounds do the work, and UPX's own invariants make them exact:
- all blocks together reconstruct `p_filesize`, so a running remainder caps the sum.
Bounding only per-block would leave the total at (block count x original size), so
the remainder is the part that actually closes it
- `p_filesize` sizes the output buffer, so it is capped both absolutely and against
the size of the file on disk. The absolute cap alone left a ~60 byte header able to
claim 500MB; the ratio alone cannot work either, since LZMA encodes a run of N
equal bytes in O(log N) and a `go:embed` of 120MB of zeros legitimately packs 209x
The output buffer is also allocated on the first block that survives validation
rather than up front, so a header claiming a large size with nothing decodable behind
it costs nothing.
Alongside that, several ways the block loop could be steered off its own buffer:
- `parseELFPTLoadOffsets` bounds-checked program headers with `phStart+phentsize >
len(buf)`, which overflows for a `p_offset` near 2^64 and lets an out-of-range entry
through into a slice index. Switched to the subtraction form, and a `phentsize`
shorter than an ELF64 program header is rejected since the reads use fixed offsets
- block placement had the same overflow shape, and on failure it skipped the copy and
fell through. `outputOffset` derives from the rejected offset, so a `p_offset` at the
uint64 ceiling wrapped it to 0 and the next block landed on the reconstructed ELF
header, still returning success. Out-of-range placement now ends the block chain,
keeping the blocks already placed since those often carry `.go.buildinfo`
- the UPX 2-byte LZMA header carries `lc`/`lp` as nibbles and `pb` as three bits, so
they can hold values LZMA does not permit. Folded into the props byte they wrap
(`lc=15, lp=15, pb=7` gives 465, which truncates to 209) and the stream mis-decodes
instead of failing
- blocks decode directly into a slice of the output buffer instead of a per-block
buffer that is then copied, so one block no longer doubles peak memory
- the block count is capped: the size budget alone still allows millions of 12-byte
blocks, each spinning up an LZMA reader
Verified against the `image-small-upx` fixture, a real `upx --best --lzma` binary:
four blocks, max `sz_unc` equal to `p_blocksize` exactly, blocks summing to 0.68 of
`p_filesize`, and a 1.96x expansion against the packed file, so every bound holds with
headroom on genuine output.
Note `p_blocksize` is deliberately not used as a ceiling on `sz_unc`. It adds nothing
the remainder does not already cover, and real output sits at exactly `p_blocksize`,
so it would run with zero headroom against a value the format does not promise.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(golang): report UPX binaries we cannot unpack as unknowns
A packed Go binary we fail to unpack means its packages are silently missing from the
SBOM. `getBuildInfo` checked the decompression error for nil and discarded it, so a
rejected file surfaced only the original `buildinfo.Read` error, and that one is
deliberately silenced because it is usually just "not a Go binary".
`decompressUPX` now marks the cases worth reporting with `errUPXDecompress`: it got
past the header and the method dispatch and still could not unpack the file. Everything
else stays quiet, which matters more than it sounds. `upx` defaults to NRV2B unless
`--lzma` is passed and only LZMA is implemented here, so treating any decompression
failure as reportable would attach a golang-cataloger unknown to every packed non-Go
binary in an image. Making the reportable case opt in rather than the quiet case a
growing list also means a guard added later is silent by default.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* refactor(golang): drop the vendored xcoff parser
The only thing the golang cataloger read from it was `TargetMachine`, which is the same
two magic bytes `getGOARCHFromBin` had already matched on to dispatch there. So a full
XCOFF walk (string table, symbol table, every relocation table) ran to recover a value
that was already in hand.
`getGOARCHFromBin` reads those two bytes directly now. That is also strictly more
correct: `xcoff.NewFile` bailed with "no symbol table" when `symptr == 0`, so a stripped
XCOFF binary reported no arch at all.
Fixtures move to `testdata/xcoff/`, since they are no longer a package's own testdata.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* feat(internal/file): report a reader's real size behind one helper
Bounds get written as "does this claim exceed what the reader actually holds", and the
subtlety is that a reader's own answer cannot be trusted: an `io.SectionReader` reports
the nominal length it was built with, so one constructed with `1<<63-1` will happily
claim to hold 8EB. Confirming the last byte is readable is what separates a real size
from a nominal one.
Returns `(int64, bool)` rather than a bare size, since there are five ways to not know
and collapsing them to 0 makes every caller reinvent "0 means stop bounding".
The doc also names the wrapper trap: a type embedding `io.ReaderAt` as an interface
promotes only `ReadAt`, so it answers no size at all however large the reader beneath it
is, and a bound written against it silently does nothing.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(golang): bound every section a reader can expand, not just the name table
`CheckSectionNameTable` bounds the one section `elf.NewFile` expands while parsing, which
is enough for a caller that only parses. It is not enough for one that goes on to read
sections: those are expanded lazily, so a 260KB ELF declaring a compressed `.symtab`
drove 1.3GB of allocation through `goversion`, which opens the file with `debug/elf`
inside its own package and cannot be routed through `elfutil.NewFile`.
`CheckAllSections` is that gate, and is deliberately named as the superset so the
relationship to the narrow one is structural rather than documented.
Also exports `ErrDeclaredSizeExceeded`. A refusal here costs the SBOM a package, so
callers report it as an unknown, and matching on an error string to decide that is not
something to build a reporting policy on.
A ruleguard rule now flags `buildinfo.Read` and `version.ReadExeFromReader` outside
`scan_binary.go`. Both open `debug/elf` internally and are only bounded there by the
wrappers that gate the reader first, which the existing rule on `elf.NewFile` could not
see through.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* feat(internal/spillbuf): sparse offset-addressed buffer that spills to disk
For output that arrives out of order, at offsets the input itself declares: a
decompressor placing extents, an archive rebuilding a file from chunks. A plain `[]byte`
cannot do that without either pre-sizing to a length the input claims or growing to the
furthest offset it names, and both hand a hostile input an allocation knob.
The load-bearing property is that it reports only what it actually stored. `Size` is the
contiguous run written from offset zero and a read past it is `io.EOF`, never the zeros
an unwritten region would hand back for free. Sparse storage serving holes as real
content is what lets a write offset stand in for output nothing produced, so anything
sizing an allocation against this reader (`saferio` in `debug/elf` does exactly that)
stays bounded by work actually done.
`FirstGap` answers where the next write fits, so callers filling holes do not need the
extent bookkeeping and cannot re-derive that rule wrongly, and it only reports gaps
lying entirely inside `[0, within)`. The extent type is unexported for the same reason.
The rest of the contract worth stating: reads and writes after `Close` fail with
`os.ErrClosed` rather than looking like an empty buffer, and a write that fails leaves
the buffer unchanged.
Memory is bounded by the limit rather than by how much is written: filling 1MB and
filling 64MB cost about the same. What lands on disk past that limit is the caller's
bound to set, not this package's. Growth is geometric and capped, because sizing the
tier to exactly what each write needs is quadratic once a caller streams through it in
small chunks.
The golang cataloger is the only consumer today, and nothing else in the tree has this
shape.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(golang): rebuild UPX binaries through spillbuf and read the reconstruction everywhere
Two amplification paths were still reachable. A 128 byte input allocated 16MB by
trusting `sz_cpr` before weighing it against the bytes the file actually holds, and the
reconstruction was assembled in a `[]byte` sized from the header's declared original
size, so a 301KB input allocated 300MB.
The reconstruction is a `spillbuf.Buffer` now, which is what makes the second one go
away: blocks are placed at the offsets the file declares, nothing is pre-sized from a
claim, and the reader's length is the contiguous run actually rebuilt. That last part is
the invariant everything downstream leans on. A reader reporting the declared size would
serve the holes between placed blocks as zeros it never stored, and a 64 byte block
parked at `p_filesize-64` then made a 300KB input hand out a 64MB reader: measured, that
drove 269MB through `getBuildInfo` and returned nothing, silently.
Placement and storage are separate concerns. `upx.go` places blocks at the offsets the
file declares and asks the buffer how much was rebuilt and where the next gap is; it
knows nothing about temp files or memory limits.
The remaining bounds are ratios against the input wherever they can be, since an
absolute ceiling only ever drops a large legitimate binary. The one absolute left is on
`p_filesize`, because padding an input is nearly free, and the LZMA dictionary is paid
for in its own block's `sz_cpr`.
Interface change: `scanFile` takes a context and hands back the reconstruction for each
binary that turned out to be packed, and the caller owns it. Everything after the build
info read (crypto settings, arch, symbols, the version scan) reads it, so a packed
binary no longer reports its packages with no symbols at all.
Also narrows what gets reported. This cataloger runs on every executable in an image, so
reporting every `getBuildInfo` error attached an unknown to every corrupt ELF, odd PE and
non-Go Mach-O slice in it. Now only four gaps, the ones that cost the SBOM something
syft could have had: a packed file we could not decode (`errUPXDecompress`), a header
the size bounds refused (`errUPXSizeRefused`), a reconstruction that came up short
(`errUPXPartial`), and an ELF expansion `elfutil` declined
(`ErrDeclaredSizeExceeded`). An unimplemented UPX method stays quiet on purpose: `upx`
defaults to NRV2B and only LZMA is implemented here, so reporting it would attach an
unknown to most packed non-Go binaries in an image.
That filtering is a new pattern. No other cataloger picks which of its errors become
unknowns, and the reason this one does is that it runs on every executable in an image
rather than on files a glob already selected.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* chore: ignore local spec/ planning notes
`/specs` was already ignored; `/spec` is the same thing under the singular name and was
not, so working notes landed in commits.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* refactor(golang): name what the unpack hands back, not where it stores it
the scan's signatures said `*spillbuf.Buffer` all the way down, which told
every reader below the scan that it cares whether the reconstruction lives in
memory or on disk. it does not. two interfaces instead, each naming only what
its side actually uses:
- `unpackedContents` for the consumers: `ReaderAt`, `Closer`, and a `Size` that
is the contiguous run rebuilt from zero
- `blockSink` for the decoder, which additionally needs `WriterAt` and
`FirstGap` to place blocks
this costs one thing worth flagging. `Close` on a nil `*spillbuf.Buffer` was
nil-safe, so callers just deferred it; widened into an interface, a nil pointer
becomes a non-nil interface and the same call panics. `closeUnpacked` absorbs
that, and it is now the only way an owner releases contents. `readerFor`,
`seekerFor` and `closeUnpacked` are the three places that resolve the nil.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* docs(elfutil): say that CheckAllSections is the decompression-bomb gate
the doc led with "bounds every section syft can drive debug/elf into
decompressing", which describes the mechanism without ever naming what it is
defending against. a reader landing on the call site could not tell that this
is the decompression-bomb check.
state the attack instead: a compressed section header declares its own
decompressed size, debug/elf believes that number and allocates it on open, and
nothing forces it to match what the compressed bytes actually yield. the 260KB
ELF that drove 1.3GB of allocation now lives in the doc rather than only at the
one call site that happened to mention it.
comments only.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* docs(golang): move the overflow note onto the bounds test it describes
the "subtraction form" note sat above the phStart assignment with ten lines of
wrap analysis between it and the `if` it was talking about, so the subtraction
it names reads as missing. it is `hdrLen-phStart < phentsize`.
move it onto that test and name the form it is avoiding (`phStart+phentsize >
hdrLen`, which can overflow past the buffer end) so the referent is not several
paragraphs away.
comments only.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* chore(golang): drop the internal/ README left over from xcoff
that README existed to record where the vendored xcoff package came from and
that Go keeps it internal, which is a provenance note worth having while
third-party code sat there. dropping xcoff took the reason with it, and
repurposing it to point at gotestdata just made it redundant: gotestdata has
its own README covering why it is under internal/, why it is not testdata/, and
what belongs in each.
internal/ now holds one self-documenting directory.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
bullseye went EOL and deb.debian.org dropped it, so the elf executable
fixture image stopped building and took the whole unit test job with it
(fixture generation runs before any test does). main has been red on this
since 2026-09-08.
two separate breakages, both handled here:
- the base repo moved to `archive.debian.org`, whose Release files are
permanently expired, so `Acquire::Check-Valid-Until=false` is required
- `bullseye-security` is worse off: its index is still served but every
package in the pool behind it 404s, and it has not landed on the archive
yet. dropping it is the only option right now. these are throwaway
fixture build tools, so base bullseye versions are fine.
verified by rebuilding the image and running the fixture `verify` target,
which still reports `PASS: all security features accounted for`.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* chore(deps): pin stereoscope to the hardlink-inode-adoption branch
temporary pin to anchore/stereoscope#670 so the hardlink changes can be
exercised end to end. needs re-pinning to a release tag before merge.
pulls transitive bumps along with it: docker/cli, docker/go-connections and
gabriel-vasile/mimetype.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix: drop the image resolver hardlink workaround
stereoscope now adopts a hardlink's target at index time, so a hardlinked name
arrives already described as a regular file with the target's size, mime type
and content. the resolver-side fix from #5029 sat on top of that doing the same
job a second time, gated on a file type that no longer shows up, so both
`resolveHardLinkTarget` implementations and the extra tree walk they cost on
every `FilesByPath` are gone.
`file.NewVirtualLocationFromImage` stays as-is. it is exported and syft is v1,
so it keeps working for anyone using it, it just has no callers in syft now.
this picks up `FilesByMIMEType`, which #5029 explicitly could not fix, so
hardlinked names now reach mime-driven catalogers. on images built around
multi-call binaries (busybox and friends) that is a lot more file and executable
entries than before; distro and toolchain images move by about one entry.
one shape regresses: a hardlink stereoscope could not adopt (a link name that is
absent, empty, names a directory, names another un-adopted link, or points into
a lower layer) keeps `TypeHardLink`, and syft goes back to collapsing it onto
its target's path. no mainstream builder emits the cross-layer case.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* bump stereoscope to main
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
#5188 changed `pnpmV6LockYaml.Parse` to take a `*yaml.Node` and #5175 added
this test against the old `[]byte` signature. both were green on their own
branches, so main does not compile.
the v9 and v6 cases in the same test already go through the `yamlDocument`
helper, this is just the v5 one that was missed.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(elf): bound compressed ELF section reads
`debug/elf` takes a section's decompressed size from that section's own
compression header, and a highly compressible stream really does deliver the
bytes that header promises, so `internal/saferio` does not help: it faithfully
allocates every one of them. A 2MB input file drives `elf.NewFile` to allocate
over 10GB and return no error, which is a fatal OOM rather than a recoverable
panic.
Which sections get read is not up to the caller. `elf.NewFile` always reads the
section-name string table, and `File.Symbols` reads `.symtab` plus whatever
section its `Link` field points at, so being selective about sections is not
enough to avoid it.
New `elfutil.NewFile` is a drop-in for `elf.NewFile` that rejects a declared
decompressed size over 128MB. Every production call site goes through it, and a
ruleguard rule keeps the next one from going direct.
The check runs in two parts, since `debug/elf` expands sections at two different
times. The section-name string table is the only one `elf.NewFile` expands
itself, so it is checked against the raw bytes before the call; everything else
is expanded lazily by `(*Section).Open` and is checked after the parse, where
names, types and decompressed sizes are already resolved.
Only the sections syft can actually reach are bounded, which keeps the guard
from costing real binaries. DWARF is excluded since nothing calls `File.DWARF`,
so a large compressed `.debug_info` no longer skips the whole file, and sections
`debug/elf` will not decompress anyway (`SHF_ALLOC`, `SHT_NOBITS`) are left
alone. The legacy `.zdebug` form is matched on the section name the way
`debug/elf` gates it rather than on the `ZLIB` magic, so an ordinary section
starting with those four bytes is not mistaken for a compressed one.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix(elf): gate debug/buildinfo behind the compressed-section check
`debug/buildinfo.Read` opens ELF files with `debug/elf` itself, and `elf.NewFile`
expands the section-name string table as it parses, so the golang cataloger was
still reachable by the same bomb `elfutil` exists to stop. A 261KB fixture drove
1.4GB of allocation through `buildinfo.Read` and returned no error.
`elfutil.CheckSectionNameTable` is now exported for that case: callers that cannot
use `NewFile` because the `debug/elf` call is made for them inside another package.
Both `buildinfo.Read` call sites go through it, including the UPX-decompressed one.
Also corrects claims that did not hold up:
- the package doc's 2MB-to-10GB figure is not reachable with zlib (~1000:1), so it
now carries the measured 510KB-to-2.6GB, and names zstd's 32767:1 since that is
what makes the small inputs possible
- `.go.buildinfo` was listed as a hot-path section elfutil covers, but it is read
through `debug/buildinfo` and never touches `Section.Data`
- the graalvm comment claimed routing size rejections away from `*elf.FormatError`
improved reporting; both branches are skipped by the caller and only the
FormatError branch logs, so it did the opposite
- `sharedLibraries` logged short and truncated files as real ELF failures, since
`debug/elf` returns a bare `io.EOF` rather than an `*elf.FormatError` for those
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* added test comments around the negative cases
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* additional tests
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* better decomposition and comments
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* use a less brittle constant for error detection
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
`readerAtAdapter.ReadAt` seeks and then issues a single `Read`, which breaks the
`io.ReaderAt` contract in both directions against the squashfs reader it exists
to wrap:
- `squashfs.File.Read` copies against the decompressed block length but advances
its block cursor by the nominal block size, so a block that decompresses short
silently stops copying and returns fewer bytes with a nil error. `ReadAt`
forbids that, and callers rely on it: anything decoding a fixed-size structure
off the result gets zero padding it has no way to detect and parses it as real
data. The GraalVM PE export table and the UPX block reader both size a buffer
from a header field and then ignore `n` entirely, so a crafted image drives
them straight through the padding.
- a read landing exactly on the end of the file returns a *full* buffer paired
with `io.EOF`. `bytes.Reader.ReadAt` returns nil there, and the callers that
treat any error as fatal were written against that, so squashfs-resident
binaries sized near a read boundary were being skipped outright.
`io.ReadFull` normalizes both: it fills the buffer across short reads, and it
clears the error once the buffer is full. A genuinely short tail is reported as
`io.EOF`, which is what `ReadAt` implementations return at the end of a file, and
what the buffering branch of `GetUnionReader` already returns.
Affects squashfs-backed sources (snaps), so in practice the binary catalogers
reading structure out of executables.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* feat(golang): add extended-stdlib scope and include patterns for symbol capture
`golang.capture-symbols` decides how much symbol data lands in the SBOM for grype's reachability analysis. It's `none`, `stdlib`, or `all` today, and the useful middle is missing: `stdlib` stops at the standard library, `all` multiplies SBOM size.
A new `extended-stdlib` configurable covers stdlib plus everything under `golang.org/x/`:
```yaml
golang:
capture-symbols: extended-stdlib
```
Also, a new `capture-symbols-include` configurable for modules that are noisy in your binaries but not everyone's. It's unioned with whatever the scope selects, so it only ever widens:
```yaml
golang:
capture-symbols: extended-stdlib
capture-symbols-include:
- github.com/klauspost/**
```
Patterns are standard doublestar globs, which matters because module paths carry `/v2`-style suffixes:
```yaml
golang:
capture-symbols-include:
- github.com/klauspost/* # compress, but not compress/v2
- github.com/klauspost/** # both
- k8s.io/client-go # exact match only
```
Ordering is `none` < `stdlib` < `extended-stdlib` < `all`. The existing three values
and the `none` default are unchanged, and the include list is inert under `none`.
Presets compile into glob lists internally, so a single matcher answers "does this
module get symbols" instead of a preset branch sitting next to a separate glob branch.
An unrecognized `capture-symbols` value still falls back to `none`, but warns now
instead of doing it silently. A malformed include pattern warns and gets skipped.
One thing worth a look beyond the feature: the `Symbols` field description in the JSON
schema was wrong after this (it claimed only `all` and `stdlib` populate anything), and
that description lives in the already-published `16.1.10`. Rather than bump a version for
a sentence, `16.1.10` is amended in place and `schema/json/README.md` grows an explicit
exception for description-only changes: descriptions only, no shape change of any kind,
`$id` unchanged. Anything else still needs a bump. Happy to split that into its own PR if
you'd rather review the policy separately.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* refactor(golang): rename capture-symbols-include to capture-symbols-modules
The key's entries are go module paths, and `-include` sitting next to `capture-symbols` reads as plausibly taking symbol or package names instead. Those spellings parse and match nothing, which is quieter than the confusion `-include` was picked to avoid, so the name now says what the list holds.
`golang.CatalogerConfig.CaptureSymbolsModules` and `WithCaptureSymbolsModules` rename with it. Nothing behavioral changes; the key is new in this PR so there is no compatibility surface.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* feat(golang): match capture-symbols-modules across major version suffixes
`github.com/anchore/*` covered `github.com/anchore/syft` and silently stopped covering it the day it became `github.com/anchore/syft/v2`. The config keeps parsing, nothing warns, and symbols quietly go missing from the SBOM. Exact paths had the same hole: `github.com/klauspost/compress` did not cover `compress/v2` either, so no spelling short of `**` survived a major bump.
A major version suffix is part of a module's path but not part of its identity, so patterns are now matched against the module path both with and without it, using `module.SplitPathVersion` from `golang.org/x/mod` (already a direct dep, already used in this package for `PseudoVersion`).
```yaml
golang:
capture-symbols-modules:
- github.com/klauspost/* # compress and compress/v2
- github.com/klauspost/compress # same module at every major version
- github.com/klauspost/compress/v2 # v2 alone
```
Only a trailing suffix is a version, which is Go's own rule. In `github.com/anchore/syft/v2/thing` the `v2` is an ordinary path element naming a major subdirectory a nested module lives in, so it stays literal and `github.com/anchore/**/thing` is how you reach it. `/v0` and `/v1` are not valid suffixes and are left alone.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
The two "RACE is unset" cases only skipped the t.Setenv call, so they
inherited whatever RACE was in the environment. Running `make test` with
RACE=false exported job-wide flipped the CI-default case and failed.
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
with a cold fixture cache the integration suite builds and saves 18 docker
images across 36 sequential tests, which walks past `go test`'s default 10m
timeout and takes the fixture cache rebuild down with it. that suite now runs
`go test` directly with `-timeout=30m` (gotest.Tasks() has no timeout option),
plus `-count=1` since the built fixtures are the side effect we're actually
after and a test cache hit would skip producing them.
also adds `RACE` as one switch for the race detector across every suite:
- `RACE=false make test` drops `-race` from unit + integration and skips the
race smoke, worth doing on a cache rebuild where the wall clock is all
docker builds anyway
- `RACE=true` forces it on locally
- unset behaves as before: on in CI, off locally and on windows
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* report each hardlink as its own file when scanning images
image scans previously collapsed a set of hardlinks onto a single file, so only
one path per inode showed up in results. dir scans report every hardlink path,
which made image vs dir SBOMs of the same filesystem diverge (and produce
different SPDX `packageVerificationCode` values for packages that own hardlinked
files).
now both image resolvers (squash and all-layers) surface each hardlink at its
own path as a regular file bound to the target's content, matching dir scans.
user-facing impact:
- SBOMs for images containing hardlinks will list more `file` entries
- SPDX `packageVerificationCode` values change for affected packages, now
matching the equivalent `dir:` scan
- adds `file.NewVirtualLocationFromImage` to the public API
fixes#5019
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix busybox test assertion
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Mirrors the existing RHEL `ExtendedSupport` detector for Ubuntu so grype can
auto-enable its ESM distro channel on Pro/ESM-enabled scans.
`findUbuntuFeatures` sets `linux.Release.ExtendedSupport` when the scanned OS is
Ubuntu and any of these hold:
- an uncommented apt source (classic `.list` or DEB822 `.sources`, or the
ubuntu-advantage auth entry) referencing `esm.ubuntu.com`
- an enabled `esm-infra`/`esm-apps` service in the ubuntu-advantage
`status.json` cache
- an installed dpkg package whose version carries an `[~+]esm` pocket suffix
(the usual in-image signal, since containers are rarely Pro-attached)
The package fallback reads `s.Artifacts.Packages` from the `DetectFeatures`
SBOM callback, which runs after package cataloging, so no new plumbing was
needed. Conservative by design: plain Ubuntu and disabled/commented ESM
evidence stay false.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
the plain version pin drifted out from under us: debian mirrors only keep
the latest point release, so 8.2.31 became uninstallable once 8.2.32 shipped
and apt pulled the unpinned transitive php8.2-cli at 8.2.32. pointing apt at
snapshot.debian.org keeps the exact point release installable so the fixture
(and its test expectations) stay reproducible.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* Rework release workflow to canonical shape
Replace the custom quality-gate job with the reusable check-version-available
and check-gate workflows from anchore/workflows. Remove the phase
workflow_dispatch input; the install-script-only path is now a standalone
workflow (release-install-script.yaml) that can be triggered independently.
- add version-available and check-gate jobs using pinned anchore/workflows SHA
- remove phase input and quality-gate job
- release job now needs [check-gate, version-available]
- release-install-script job no longer conditionally skips based on phase
- add release-install-script.yaml for standalone install script runs
- set permissions: {} at workflow level (contents pushed to release job)
- add concurrency: group: release
Signed-off-by: wagoodman <wagoodman@gmail.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* Tighten workflow-level permissions to {}
Change top-level permissions from contents: read to {} in validations.yaml
and validate-github-actions.yaml, pushing the needed contents: read down
to each job that performs a checkout.
Signed-off-by: wagoodman <wagoodman@gmail.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* keep install script phase, remove workflow
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* remove schema detection workflow
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: wagoodman <wagoodman@gmail.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
The JRuby project migrated their downloads from S3 to GitHub Releases,
causing the old S3 URLs to return HTTP 403 Forbidden and breaking test
fixture image builds.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* migrate fixtures to testdata
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix: correct broken symlinks after testdata migration
The migration from test-fixtures to testdata broke several symlinks:
- elf-test-fixtures symlinks pointed to old test-fixtures paths
- elf-test-fixtures needed to be renamed to elf-testdata
- image-pkg-coverage symlink pointed to test-fixtures instead of testdata
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix: handle missing classifiers/bin directory in Makefile
The clean-fingerprint target was failing when classifiers/bin doesn't
exist (e.g., on fresh clone without downloaded binaries).
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix: add gitignore negation for jar/zip fixtures in test/cli
The jar and zip files in test/cli/testdata/image-unknowns were being
gitignored by the root .gitignore patterns. This caused them to be
untracked and not included when building docker images in CI, resulting
in Test_Unknowns failures since the test expects errors from corrupt
archive files that weren't present.
Add a .gitignore in test/cli/testdata to negate the exclusions for
these specific test fixture files.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* switch fixture cache to v2
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* test: update expected versions for rebuilt fixtures
Update test expectations for packages that have been updated in
upstream repositories when docker images are rebuilt:
- glibc: 2.42-r4 → 2.43-r1 (wolfi)
- php: 8.2.29 → 8.2.30 (ubuntu/apache)
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* upgrade go
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix: add go-shlex dependency for testdata manager tool
The manager tool in syft/pkg/cataloger/binary/testdata/ imports
go-shlex, but since it's in a testdata directory, Go doesn't track
its dependencies. This caused CI failures when go.mod didn't
explicitly list the dependency.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* refactor: move binary classifier manager to internal/
Move the manager tool from testdata/manager to internal/manager so
that Go properly tracks its dependencies. Code in testdata directories
is ignored by Go for dependency tracking, which caused CI failures
when go.mod didn't explicitly list transitive dependencies.
This is a cleaner solution than manually adding dependencies to go.mod
for code that happens to live in testdata.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* fix: add gitignore negations for test fixtures blocked by root patterns
Multiple test fixtures were being blocked by root-level gitignore patterns
like bin/, *.jar, *.tar, and *.exe. This adds targeted .gitignore files with
negation patterns to allow these specific test fixtures to be tracked:
- syft/linux/testdata/os/busybox/bin/busybox (blocked by bin/)
- syft/pkg/cataloger/java/testdata/corrupt/example.{jar,tar} (blocked by *.jar, *.tar)
- syft/pkg/cataloger/binary/testdata/classifiers/snippets/go-version-hint/**/bin/go (blocked by bin/)
- syft/pkg/cataloger/bitnami/testdata/no-rel/.../bin/redis-server (blocked by bin/)
Also updates the bitnami test expectation to include the newly required
.gitignore files in the test fixture.
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* test: update glibc version expectation (2.43-r1 -> 2.43-r2)
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* add capability drift check as unit step
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* dont clear test observations before drift detection
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
* bump stereoscope commit to main
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
---------
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Alex Goodman
