diff --git a/syft/pkg/cataloger/java/graalvm_native_image_cataloger.go b/syft/pkg/cataloger/java/graalvm_native_image_cataloger.go index 5ed5801b5..ab0845360 100644 --- a/syft/pkg/cataloger/java/graalvm_native_image_cataloger.go +++ b/syft/pkg/cataloger/java/graalvm_native_image_cataloger.go @@ -14,6 +14,7 @@ import ( "unsafe" "github.com/anchore/syft/internal" + intFile "github.com/anchore/syft/internal/file" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/internal/mimetype" "github.com/anchore/syft/syft/artifact" @@ -63,10 +64,9 @@ type exportContentPE struct { addressOfSvmVersion uint32 } -// A nativeImagePE must maintain the underlying reader to fetch information unavailable in the Golang API. +// nativeImagePE carries the raw export directory, which debug/pe does not expose. type nativeImagePE struct { file *pe.File - reader io.ReaderAt exportSymbols pe.DataDirectory exports []byte t exportTypesPE @@ -83,6 +83,16 @@ const nativeImageMissingSymbolsError = "one or more symbols are missing from the const nativeImageInvalidIndexError = "parsing the executable file generated an invalid index" const nativeImageMissingExportedDataDirectoryError = "exported data directory is missing" +// nativeImageMaxDecompressedSbomSize bounds the decompressed SBOM rather than the compressed bytes, +// since gzip turns a few kilobytes into gigabytes. Real SBOMs run to single-digit MB even at 20k +// components, and decoding costs several times that again in graph allocation. +const nativeImageMaxDecompressedSbomSize = 16 * intFile.MB + +// nativeImageMaxExportDirectorySize bounds the export directory, which is symbol metadata and runs to a +// few KB. Bounding by the bytes remaining is not enough on its own: a mostly empty multi-gigabyte file +// is cheap to ship in a layer and would still authorize reading all of it. +const nativeImageMaxExportDirectorySize = 16 * intFile.MB + // NewNativeImageCataloger returns a new Native Image cataloger object. func NewNativeImageCataloger() pkg.Cataloger { return &nativeImageCataloger{} @@ -94,35 +104,40 @@ func (c *nativeImageCataloger) Name() string { } // decompressSbom returns the packages given within a native image executable's SBOM. -func decompressSbom(dataBuf []byte, sbomStart uint64, lengthStart uint64) ([]pkg.Package, []artifact.Relationship, error) { - lengthEnd := lengthStart + 8 - bufLen := len(dataBuf) - if lengthEnd > uint64(bufLen) { +// +// Offsets and the stored length all come from the binary, so the bounds below subtract from the buffer +// length rather than adding to a file-controlled value, which wraps and then panics on the slice. +func decompressSbom(dataBuf []byte, sbomStart, lengthStart uint64) ([]pkg.Package, []artifact.Relationship, error) { + bufLen := uint64(len(dataBuf)) + if lengthStart > bufLen || bufLen-lengthStart < 8 { return nil, nil, errors.New("the 'sbom_length' symbol overflows the binary") } - length := dataBuf[lengthStart:lengthEnd] - p := bytes.NewBuffer(length) - var storedLength uint64 - err := binary.Read(p, binary.LittleEndian, &storedLength) - if err != nil { - return nil, nil, fmt.Errorf("could not read from binary file: %w", err) - } + storedLength := binary.LittleEndian.Uint64(dataBuf[lengthStart : lengthStart+8]) log.WithFields("len", storedLength).Trace("found java native-image SBOM") - sbomEnd := sbomStart + storedLength - if sbomEnd > uint64(bufLen) { + if sbomStart > bufLen || storedLength > bufLen-sbomStart { return nil, nil, errors.New("the sbom symbol overflows the binary") } - sbomCompressed := dataBuf[sbomStart:sbomEnd] - p = bytes.NewBuffer(sbomCompressed) - gzreader, err := gzip.NewReader(p) + sbomCompressed := dataBuf[sbomStart : sbomStart+storedLength] + gzreader, err := gzip.NewReader(bytes.NewBuffer(sbomCompressed)) if err != nil { return nil, nil, fmt.Errorf("could not decompress the java native-image SBOM: %w", err) } + defer gzreader.Close() - sbom, _, _, err := cyclonedxjson.NewFormatDecoder().Decode(gzreader) + // the compressed bytes are bounded by the file, the decompressed stream is not. one byte past the + // ceiling, so a stream sitting exactly on it is still legal + raw, err := io.ReadAll(io.LimitReader(gzreader, nativeImageMaxDecompressedSbomSize+1)) + if err != nil { + return nil, nil, fmt.Errorf("could not decompress the java native-image SBOM: %w", err) + } + if len(raw) > nativeImageMaxDecompressedSbomSize { + return nil, nil, fmt.Errorf("the java native-image SBOM decompresses past %d bytes", nativeImageMaxDecompressedSbomSize) + } + + sbom, _, _, err := cyclonedxjson.NewFormatDecoder().Decode(bytes.NewReader(raw)) if err != nil { return nil, nil, fmt.Errorf("could not unmarshal the java native-image SBOM: %w", err) } @@ -133,6 +148,15 @@ func decompressSbom(dataBuf []byte, sbomStart uint64, lengthStart uint64) ([]pkg return pkgs, sbom.Relationships, nil } +// symbolOffset converts an SBOM symbol address into an offset within the section that should hold it. +// The subtraction is unsigned, so an address below the section base underflows into a huge offset. +func symbolOffset(addr, sectionBase uint64) (uint64, error) { + if addr < sectionBase { + return 0, errors.New("an SBOM symbol precedes the section that should contain it") + } + return addr - sectionBase, nil +} + // fileError logs an error message when an executable cannot be read. func fileError(filename string, err error) (nativeImage, error) { // We could not read the file as a binary for the desired platform, but it may still be a native-image executable. @@ -174,6 +198,8 @@ func newMachO(filename string, r io.ReaderAt) (nativeImage, error) { log.WithFields("filename", filename, "error", err).Trace("not a MachO binary") return nil, nil } + // a real read failure, not a format mismatch; returning nil here would drop the file silently + return fileError(filename, err) } if bi == nil { return nil, nil @@ -183,6 +209,35 @@ func newMachO(filename string, r io.ReaderAt) (nativeImage, error) { }, nil } +// readExportDirectory weighs the size the file declares against both an absolute cap and the bytes +// really present before allocating. +// +// note: dir.VirtualAddress is an RVA used directly as a file offset, with no section-table translation. +// Long-standing behavior this preserves rather than fixes; see rvaToFileOffset in dotnet/pe. +func readExportDirectory(r io.ReaderAt, dir pe.DataDirectory) ([]byte, error) { + if dir.Size > nativeImageMaxExportDirectorySize { + return nil, fmt.Errorf("export directory declares %d bytes, over the %d byte limit", dir.Size, nativeImageMaxExportDirectorySize) + } + + // the size has to come from the reader, not from what the file claims + end, ok := intFile.ReaderSize(r) + if !ok { + return nil, errors.New("unable to measure the binary") + } + if remaining := end - int64(dir.VirtualAddress); remaining < int64(dir.Size) { + return nil, fmt.Errorf("export directory declares %d bytes at RVA %d but only %d remain", + dir.Size, dir.VirtualAddress, max(remaining, 0)) + } + + // the count decides, not the error: a ReadAt may report a full read as io.EOF, or a short one with no + // error at all. io.ReadFull covers both but spins forever on a reader that keeps answering (0, nil) + exports := make([]byte, dir.Size) + if n, err := r.ReadAt(exports, int64(dir.VirtualAddress)); n < len(exports) { + return nil, fmt.Errorf("could not read the exported symbols data directory: read %d of %d bytes (%v)", n, len(exports), err) + } + return exports, nil +} + // newPE reads a Native Image from a Portable Executable file. func newPE(filename string, r io.ReaderAt) (nativeImage, error) { // First attempt to read an PE file. @@ -211,15 +266,12 @@ func newPE(filename string, r io.ReaderAt) (nativeImage, error) { if exportSymbolsDataDirectory.Size == 0 { return fileError(filename, errors.New(nativeImageMissingExportedDataDirectoryError)) } - exportSymbolsOffset := uint64(exportSymbolsDataDirectory.VirtualAddress) - exports := make([]byte, exportSymbolsDataDirectory.Size) - _, err = r.ReadAt(exports, int64(exportSymbolsOffset)) + exports, err := readExportDirectory(r, exportSymbolsDataDirectory) if err != nil { - return fileError(filename, fmt.Errorf("could not read the exported symbols data directory: %w", err)) + return fileError(filename, err) } return nativeImagePE{ file: bi, - reader: r, exportSymbols: exportSymbolsDataDirectory, exports: exports, t: exportTypesPE{ @@ -275,15 +327,20 @@ func (ni nativeImageElf) fetchPkgs() (pkgs []pkg.Package, relationships []artifa } dataSection := bi.Section(".data") if dataSection == nil { - return nil, nil, fmt.Errorf("no .data section found in binary: %w", err) + return nil, nil, errors.New("no .data section found in binary") } - dataSectionBase := dataSection.Addr data, err := dataSection.Data() if err != nil { return nil, nil, fmt.Errorf("cannot read the .data section: %w", err) } - sbomLocation := sbom.Value - dataSectionBase - lengthLocation := sbomLength.Value - dataSectionBase + sbomLocation, err := symbolOffset(sbom.Value, dataSection.Addr) + if err != nil { + return nil, nil, err + } + lengthLocation, err := symbolOffset(sbomLength.Value, dataSection.Addr) + if err != nil { + return nil, nil, err + } return decompressSbom(data, sbomLocation, lengthLocation) } @@ -354,48 +411,43 @@ func (ni nativeImageMachO) fetchPkgs() (pkgs []pkg.Package, relationships []arti log.Tracef("cannot obtain buffer from data segment") return nil, nil, nil } - sbomLocation := sbom.Value - dataSegment.Addr - lengthLocation := sbomLength.Value - dataSegment.Addr + sbomLocation, err := symbolOffset(sbom.Value, dataSegment.Addr) + if err != nil { + return nil, nil, err + } + lengthLocation, err := symbolOffset(sbomLength.Value, dataSegment.Addr) + if err != nil { + return nil, nil, err + } return decompressSbom(dataBuf, sbomLocation, lengthLocation) } // fetchExportAttribute obtains an attribute from the exported symbols directory entry. func (ni nativeImagePE) fetchExportAttribute(i int) (uint32, error) { - var attribute uint32 n := len(ni.exports) - j := int(unsafe.Sizeof(ni.header)) + i*int(unsafe.Sizeof(ni.t.headerAttribute)) - if j+4 >= n { + sz := int(unsafe.Sizeof(ni.t.headerAttribute)) + // i is only ever 0-3, so this cannot overflow; > not >= because an attribute ending flush with the + // directory is still entirely present + j := int(unsafe.Sizeof(ni.header)) + i*sz + if j+sz > n { log.Tracef("invalid index to export directory entry attribute: %v", j) return uint32(0), errors.New(nativeImageInvalidIndexError) } - p := bytes.NewBuffer(ni.exports[j : j+4]) - err := binary.Read(p, binary.LittleEndian, &attribute) - if err != nil { - log.Tracef("error fetching export directory entry attribute: %v", err) - return uint32(0), err - } - return attribute, nil + return binary.LittleEndian.Uint32(ni.exports[j : j+sz]), nil } // fetchExportFunctionPointer obtains a function pointer from the exported symbols directory entry. func (ni nativeImagePE) fetchExportFunctionPointer(functionsBase uint32, i uint32) (uint32, error) { - var pointer uint32 - - n := uint32(len(ni.exports)) - sz := uint32(unsafe.Sizeof(ni.t.functionPointer)) - j := functionsBase + i*sz - if j+sz >= n { + // functionsBase is a file-controlled RVA: in uint32 the sum wraps under the bound, then panics + n := uint64(len(ni.exports)) + sz := uint64(unsafe.Sizeof(ni.t.functionPointer)) + j := uint64(functionsBase) + uint64(i)*sz + if j > n || n-j < sz { log.Tracef("invalid index to exported function: %v", j) return uint32(0), errors.New(nativeImageInvalidIndexError) } - p := bytes.NewBuffer(ni.exports[j : j+sz]) - err := binary.Read(p, binary.LittleEndian, &pointer) - if err != nil { - log.Tracef("error fetching exported function: %v", err) - return uint32(0), err - } - return pointer, nil + return binary.LittleEndian.Uint32(ni.exports[j : j+sz]), nil } // fetchExportContent obtains the content of the export directory entry relevant to the SBOM. @@ -427,27 +479,30 @@ func (ni nativeImagePE) fetchSbomSymbols(content *exportContentPE) { sbomBytes := []byte(nativeImageSbomSymbol + "\x00") sbomLengthBytes := []byte(nativeImageSbomLengthSymbol + "\x00") svmVersionInfoBytes := []byte(nativeImageSbomVersionSymbol + "\x00") - n := uint32(len(ni.exports)) + n := uint64(len(ni.exports)) + sz := uint64(unsafe.Sizeof(ni.t.namePointer)) + + // an RVA below the directory would underflow into a huge offset that wraps back into range below + if content.addressOfNames < ni.exportSymbols.VirtualAddress { + log.Tracef("exported name array precedes the export directory: %v", content.addressOfNames) + return + } + addressBase := uint64(content.addressOfNames - ni.exportSymbols.VirtualAddress) // Find SBOM, SBOM Length, and SVM Version Symbol for i := uint32(0); i < content.numberOfNames; i++ { - j := i * uint32(unsafe.Sizeof(ni.t.namePointer)) - addressBase := content.addressOfNames - ni.exportSymbols.VirtualAddress - k := addressBase + j - sz := uint32(unsafe.Sizeof(ni.t.namePointer)) - if k+sz >= n { + k := addressBase + uint64(i)*sz + if k > n || n-k < sz { log.Tracef("invalid index to exported function: %v", k) // If we are at the end of exports, stop looking return } - var symbolAddress uint32 - p := bytes.NewBuffer(ni.exports[k : k+sz]) - err := binary.Read(p, binary.LittleEndian, &symbolAddress) - if err != nil { - log.Tracef("error fetching address of symbol %v", err) + symbolAddress := binary.LittleEndian.Uint32(ni.exports[k : k+sz]) + if symbolAddress < ni.exportSymbols.VirtualAddress { + log.Tracef("exported symbol precedes the export directory: %v", symbolAddress) return } - symbolBase := symbolAddress - ni.exportSymbols.VirtualAddress + symbolBase := uint64(symbolAddress - ni.exportSymbols.VirtualAddress) if symbolBase >= n { log.Tracef("invalid index to exported symbol: %v", symbolBase) return @@ -482,6 +537,9 @@ func (ni nativeImagePE) fetchPkgs() (pkgs []pkg.Package, relationships []artifac if content.addressOfSbom == uint32(0) || content.addressOfSbomLength == uint32(0) || content.addressOfSvmVersion == uint32(0) { return nil, nil, errors.New(nativeImageMissingSymbolsError) } + if content.addressOfFunctions < ni.exportSymbols.VirtualAddress { + return nil, nil, errors.New("exported function array precedes the export directory") + } functionsBase := content.addressOfFunctions - ni.exportSymbols.VirtualAddress sbomOffset := content.addressOfSbom sbomAddress, err := ni.fetchExportFunctionPointer(functionsBase, sbomOffset) @@ -503,10 +561,16 @@ func (ni nativeImagePE) fetchPkgs() (pkgs []pkg.Package, relationships []artifac log.Tracef("cannot obtain buffer from the java native-image .data section") return nil, nil, nil } - sbomLocation := sbomAddress - dataSection.VirtualAddress - lengthLocation := sbomLengthAddress - dataSection.VirtualAddress + sbomLocation, err := symbolOffset(uint64(sbomAddress), uint64(dataSection.VirtualAddress)) + if err != nil { + return nil, nil, err + } + lengthLocation, err := symbolOffset(uint64(sbomLengthAddress), uint64(dataSection.VirtualAddress)) + if err != nil { + return nil, nil, err + } - return decompressSbom(dataBuf, uint64(sbomLocation), uint64(lengthLocation)) + return decompressSbom(dataBuf, sbomLocation, lengthLocation) } // fetchPkgs provides the packages available in a UnionReader. @@ -527,6 +591,9 @@ func fetchPkgs(reader unionreader.UnionReader, location file.Location) ([]pkg.Pa for _, makeNativeImage := range imageFormats { ni, err := makeNativeImage(filename, r) if err != nil { + // both "not this format" and a real rejection of a file that is one, which drops every + // package the binary carries and is only visible at trace level + log.WithFields("file", filename, "error", err).Trace("unable to read possible java native-image") continue } if ni == nil { @@ -535,14 +602,17 @@ func fetchPkgs(reader unionreader.UnionReader, location file.Location) ([]pkg.Pa newPkgs, newRelationships, err := ni.fetchPkgs() if err != nil { log.Tracef("unable to extract SBOM from possible java native-image %s: %v", filename, err) - continue + } else { + // Associate extracted packages with the native image location + for i := range newPkgs { + newPkgs[i].Locations.Add(location) + } + pkgs = append(pkgs, newPkgs...) + relationships = append(relationships, newRelationships...) } - // Associate extracted packages with the native image location - for i := range newPkgs { - newPkgs[i].Locations.Add(location) - } - pkgs = append(pkgs, newPkgs...) - relationships = append(relationships, newRelationships...) + // nothing parses as two of these today; stopping makes that explicit rather than relying on it. + // note a format that parses but whose fetchPkgs fails is not retried as another format + break } } return pkgs, relationships diff --git a/syft/pkg/cataloger/java/graalvm_native_image_cataloger_test.go b/syft/pkg/cataloger/java/graalvm_native_image_cataloger_test.go index 504b6f16b..67f0fb212 100644 --- a/syft/pkg/cataloger/java/graalvm_native_image_cataloger_test.go +++ b/syft/pkg/cataloger/java/graalvm_native_image_cataloger_test.go @@ -4,15 +4,19 @@ import ( "bufio" "bytes" "compress/gzip" + "crypto/sha256" "encoding/binary" "fmt" "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/syft/artifact" "github.com/anchore/syft/syft/cpe" "github.com/anchore/syft/syft/internal/unionreader" @@ -265,3 +269,303 @@ func verifyRelationshipFields(t *testing.T, expected, actual []artifact.Relation require.Equal(t, expected[i].Type, actual[i].Type) } } + +// buildMinimalPE64 assembles the smallest PE64 debug/pe will parse, with DataDirectory[0] set to the +// given RVA and size. The point of the fixture is that size can claim far more than totalSize. +// +// newPE uses the directory's VirtualAddress directly as a file offset, so no sections are needed and +// exportRVA doubles as the offset the read starts from. +func buildMinimalPE64(exportRVA, exportSize uint32, totalSize int) []byte { + const peHdrOff = 0x40 + buf := make([]byte, totalSize) + copy(buf, []byte{'M', 'Z'}) + binary.LittleEndian.PutUint32(buf[0x3c:], peHdrOff) + + p := buf[peHdrOff:] + copy(p, []byte{'P', 'E', 0, 0}) + // COFF file header + binary.LittleEndian.PutUint16(p[4:], 0x8664) // Machine = AMD64 + binary.LittleEndian.PutUint16(p[20:], 240) // SizeOfOptionalHeader + binary.LittleEndian.PutUint16(p[22:], 0x22) // Characteristics: executable image + + // optional header (PE32+) + oh := p[24:] + binary.LittleEndian.PutUint16(oh[0:], 0x20b) // Magic = PE32+ + binary.LittleEndian.PutUint32(oh[108:], 16) // NumberOfRvaAndSizes + // DataDirectory[0] = export table + binary.LittleEndian.PutUint32(oh[112:], exportRVA) + binary.LittleEndian.PutUint32(oh[116:], exportSize) + + 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 { + *bytes.Reader + maxRead int +} + +func (r *readSizeRecorder) Read(p []byte) (int, error) { + if len(p) > r.maxRead { + r.maxRead = len(p) + } + return r.Reader.Read(p) +} + +func (r *readSizeRecorder) ReadAt(p []byte, off int64) (int, error) { + if len(p) > r.maxRead { + r.maxRead = len(p) + } + return r.Reader.ReadAt(p, off) +} + +func TestNewPE_ExportDirectorySizeIsNotAllocatedUpFront(t *testing.T) { + // Size is a user-controlled uint32 that used to size a make([]byte, Size) before the read, so a small + // file claiming 4GB reserved 4GB. + const totalSize = 8192 + data := buildMinimalPE64(0x1000, 0xFFFFFFFF, totalSize) + r := &readSizeRecorder{Reader: bytes.NewReader(data)} + + var err error + allocated := measureAlloc(t, func() { + _, err = newPE("oversized-export-dir.exe", r) + }) + + require.Error(t, err, "an export directory the file cannot satisfy must not read as successful") + require.ErrorContains(t, err, "byte limit") + + t.Logf("allocated %d bytes for a directory declaring %d in a %d byte file", + allocated, uint64(0xFFFFFFFF), totalSize) + require.Less(t, allocated, uint64(intFile.MB), + "the declared size must not be reserved before the read confirms the bytes exist") + assert.LessOrEqual(t, r.maxRead, totalSize, + "no single read may exceed the file size, regardless of what the headers claim") +} + +func TestNewPE_ExportDirectoryWithinFileIsRead(t *testing.T) { + // exportRVA=0x1000 in an 8192 byte file leaves exactly 4096 bytes for the directory to claim + const exportRVA = 0x1000 + const totalSize = 8192 + const remaining = totalSize - exportRVA + + tests := []struct { + name string + exportSize uint32 + wantErr bool + }{ + {name: "well within the file", exportSize: 64, wantErr: false}, + // pins the boundary the not-enough-bytes check must not break: every byte remaining is available + {name: "exactly the bytes remaining", exportSize: remaining, wantErr: false}, + {name: "one byte past what remains", exportSize: remaining + 1, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data := buildMinimalPE64(exportRVA, tt.exportSize, totalSize) + + ni, err := newPE("ok.exe", bytes.NewReader(data)) + if tt.wantErr { + require.Error(t, err, "the bound must not allow reading past what the file has") + return + } + require.NoError(t, err, "the bound must not reject an export directory that really is present") + require.NotNil(t, ni) + }) + } +} + +func TestDecompressSbom_RejectsOutOfRangeOffsets(t *testing.T) { + // every offset and length here is read out of the binary, so each must be rejected rather than wrapped + // into a value that passes a bound and panics on the slice. wantErrMsg pins which of the three guards fires + const sbomLengthOverflowsMsg = "the 'sbom_length' symbol overflows the binary" + const sbomOverflowsMsg = "the sbom symbol overflows the binary" + + tests := []struct { + name string + bufLen int + sbomStart uint64 + lengthStart uint64 + storedLength uint64 + wantErrMsg string + }{ + { + name: "length offset wraps past the end of the buffer", + bufLen: 64, + lengthStart: ^uint64(0) - 3, // lengthStart+8 wraps to 4, which compares as in range + wantErrMsg: sbomLengthOverflowsMsg, + }, + { + name: "length offset starts beyond the buffer", + bufLen: 64, + lengthStart: 100, + wantErrMsg: sbomLengthOverflowsMsg, + }, + { + name: "stored length wraps past the end of the buffer", + bufLen: 64, + sbomStart: 16, + lengthStart: 0, + storedLength: ^uint64(0) - 3, // sbomStart+storedLength wraps to 12 + wantErrMsg: sbomOverflowsMsg, + }, + { + name: "stored length runs past the end of the buffer", + bufLen: 64, + sbomStart: 16, + lengthStart: 0, + storedLength: 1000, + wantErrMsg: sbomOverflowsMsg, + }, + { + name: "sbom offset starts beyond the buffer", + bufLen: 64, + sbomStart: ^uint64(0) - 3, + lengthStart: 0, + wantErrMsg: sbomOverflowsMsg, + }, + { + name: "length field straddles the end of the buffer", + bufLen: 64, + lengthStart: 60, + wantErrMsg: sbomLengthOverflowsMsg, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dataBuf := make([]byte, tt.bufLen) + // subtract rather than add, for the same reason the code under test has to: lengthStart is deliberately + // large enough to wrap + if tt.lengthStart <= uint64(tt.bufLen) && uint64(tt.bufLen)-tt.lengthStart >= 8 { + binary.LittleEndian.PutUint64(dataBuf[tt.lengthStart:], tt.storedLength) + } + + var err error + require.NotPanics(t, func() { + _, _, err = decompressSbom(dataBuf, tt.sbomStart, tt.lengthStart) + }) + require.ErrorContains(t, err, tt.wantErrMsg) + }) + } +} + +func TestDecompressSbom_RejectsStreamExpandingPastTheLimit(t *testing.T) { + // zeros stand in for any uniform, highly compressible payload: they expand about 1000x, far past what + // real JSON does + tests := []struct { + name string + decompressedSize int + wantRejected bool + }{ + { + name: "one byte past the limit is rejected", + decompressedSize: nativeImageMaxDecompressedSbomSize + 1, + wantRejected: true, + }, + { + // pins the other side: tightening the LimitedReader by one byte would reject a legal payload + name: "exactly at the limit falls through to the decoder", + decompressedSize: nativeImageMaxDecompressedSbomSize, + wantRejected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var compressed bytes.Buffer + z := gzip.NewWriter(&compressed) + _, err := z.Write(make([]byte, tt.decompressedSize)) + require.NoError(t, err) + require.NoError(t, z.Close()) + + t.Logf("%d compressed bytes expand to %d (%dx)", compressed.Len(), tt.decompressedSize, + tt.decompressedSize/compressed.Len()) + + dataBuf := append(compressed.Bytes(), make([]byte, 8)...) + lengthStart := uint64(compressed.Len()) + binary.LittleEndian.PutUint64(dataBuf[lengthStart:], lengthStart) + + var decompressErr error + allocated := measureAlloc(t, func() { + _, _, decompressErr = decompressSbom(dataBuf, 0, lengthStart) + }) + err = decompressErr + require.Error(t, err, "zeros are never a valid SBOM, so this errors either way") + if tt.wantRejected { + require.Contains(t, err.Error(), "decompresses past", + "hitting the bound should say so, not report a parse failure") + t.Logf("allocated %d bytes rejecting a stream past the limit", allocated) + // generous on purpose: -race roughly doubles the allocation counted here, and the point is catching an + // unbounded blowup, not pinning a multiple + require.Less(t, allocated, uint64(8*nativeImageMaxDecompressedSbomSize), + "rejecting an oversized stream must not retain multiples of the limit in memory") + return + } + require.NotContains(t, err.Error(), "decompresses past", + "a stream sitting exactly on the limit must reach the decoder, not be capped away") + }) + } +} + +// the other tests compute their sizes FROM nativeImageMaxDecompressedSbomSize, so raising it leaves them +// green. This is the one assertion that does not. +func TestNativeImageMaxDecompressedSbomSize_Ceiling(t *testing.T) { + require.LessOrEqual(t, int64(nativeImageMaxDecompressedSbomSize), int64(64*intFile.MB), + "the ceiling is what stands between a gzip bomb and an OOM; raising it needs a new argument here") +} + +func TestDecompressSbom_AcceptsLargeSbom(t *testing.T) { + // the size limit must not truncate a genuinely large SBOM. Distinct names, versions, purls and hashes + // keep this shaped like a real CycloneDX document rather than a uniform payload. + const components = 20000 + + var sb bytes.Buffer + sb.WriteString(`{"bomFormat":"CycloneDX","specVersion":"1.5","version":1,"components":[`) + for i := 0; i < components; i++ { + if i > 0 { + sb.WriteString(",") + } + // a sha256-shaped value keeps the entropy (and so the compression ratio) realistic + hash := sha256.Sum256([]byte(fmt.Sprintf("component-%d", i))) + fmt.Fprintf(&sb, `{"type":"library","name":"lib-%d","version":"%d.%d.%d",`+ + `"purl":"pkg:maven/com.example.group%d/lib-%d@%d.%d.%d",`+ + `"hashes":[{"alg":"SHA-256","content":"%x"}]}`, + i, i%20, i%7, i%13, i%50, i, i%20, i%7, i%13, hash) + } + sb.WriteString(`]}`) + + raw := sb.Bytes() + var compressed bytes.Buffer + z := gzip.NewWriter(&compressed) + _, err := z.Write(raw) + require.NoError(t, err) + require.NoError(t, z.Close()) + + t.Logf("%d components: %d bytes of JSON, %d compressed (%dx), limit %d", + components, len(raw), compressed.Len(), len(raw)/compressed.Len(), nativeImageMaxDecompressedSbomSize) + require.Less(t, int64(len(raw)), int64(nativeImageMaxDecompressedSbomSize), + "a real SBOM of this size must sit well inside the limit, otherwise the limit is too low") + + dataBuf := append(compressed.Bytes(), make([]byte, 8)...) + lengthStart := uint64(compressed.Len()) + binary.LittleEndian.PutUint64(dataBuf[lengthStart:], lengthStart) + + pkgs, _, err := decompressSbom(dataBuf, 0, lengthStart) + require.NoError(t, err, "a large SBOM at a realistic ratio must not be rejected") + require.Len(t, pkgs, components) +} diff --git a/syft/pkg/cataloger/java/graalvm_native_image_offsets_test.go b/syft/pkg/cataloger/java/graalvm_native_image_offsets_test.go new file mode 100644 index 000000000..61614f84f --- /dev/null +++ b/syft/pkg/cataloger/java/graalvm_native_image_offsets_test.go @@ -0,0 +1,145 @@ +package java + +import ( + "bytes" + "io" + "math" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/anchore/syft/syft/file" + "github.com/anchore/syft/syft/internal/unionreader" +) + +func TestSymbolOffset(t *testing.T) { + tests := []struct { + name string + addr uint64 + sectionBase uint64 + wantOffset uint64 + wantErr bool + }{ + { + name: "addr below section base underflows if unchecked, so it must error instead", + addr: 0x900, + sectionBase: 0x1000, + wantErr: true, + }, + { + name: "addr equal to section base gives offset zero", + addr: 0x1000, + sectionBase: 0x1000, + wantOffset: 0, + }, + { + name: "addr above section base gives the exact delta", + addr: 0x1010, + sectionBase: 0x1000, + wantOffset: 0x10, + }, + { + name: "zero section base succeeds for any addr", + addr: 0x2a, + sectionBase: 0, + wantOffset: 0x2a, + }, + { + name: "max addr and max section base are equal, so offset is zero", + addr: math.MaxUint64, + sectionBase: math.MaxUint64, + wantOffset: 0, + }, + { + name: "max addr against a zero section base gives the max delta", + addr: math.MaxUint64, + sectionBase: 0, + wantOffset: math.MaxUint64, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := symbolOffset(tt.addr, tt.sectionBase) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantOffset, got) + }) + } +} + +// readerFromFixture opens a testdata fixture and wraps it the same way processLocation does before +// handing it to fetchPkgs. +func readerFromFixture(t *testing.T, path string) unionreader.UnionReader { + t.Helper() + f, err := os.Open(path) + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + + reader, err := unionreader.GetUnionReader(io.NopCloser(f)) + require.NoError(t, err) + return reader +} + +// neither fixture is a native image, so both exercise the path where a format matches and its fetchPkgs +// then fails, which the break at the end of the format loop makes final. +// +// the break itself cannot be distinguished from the old fall-through here: ELF, Mach-O and PE magics are +// mutually exclusive, so no fixture can validly parse as two formats. Nothing reaches the branch that +// attaches location to each package either, since no committed fixture is a real native image with an +// embedded SBOM. +func TestFetchPkgs(t *testing.T) { + tests := []struct { + name string + fixture string + }{ + { + name: "real native image without SBOM symbols", + fixture: "testdata/java-builds/packages/example-java-app", + }, + { + name: "real Mach-O that is not a native image", + fixture: "testdata/java-builds/packages/gcc-amd64-darwin-exec-debug", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reader := readerFromFixture(t, tt.fixture) + location := file.NewLocation(tt.fixture) + + require.NotPanics(t, func() { + gotPkgs, gotRels := fetchPkgs(reader, location) + assert.Empty(t, gotPkgs) + assert.Empty(t, gotRels) + }) + }) + } +} + +// covers macho.NewFile failing with something other than *macho.FormatError. Garbage bytes take the +// FormatError branch and return (nil, nil); a valid magic with nothing behind it does not. +func TestNewMachO_TruncatedFileIsARealFailure(t *testing.T) { + // a valid Mach-O magic with nothing behind it yields io.ErrUnexpectedEOF, not a *macho.FormatError + _, err := newMachO("truncated", bytes.NewReader([]byte{0xcf, 0xfa, 0xed, 0xfe})) + require.ErrorContains(t, err, "unable to read executable") +} + +func TestFetchPkgs_GarbageBytesYieldsNothing(t *testing.T) { + // bytes that match none of ELF, Mach-O or PE magic numbers + garbage := bytes.Repeat([]byte{0xDE, 0xAD, 0xBE, 0xEF}, 256) + reader, err := unionreader.GetUnionReader(io.NopCloser(bytes.NewReader(garbage))) + require.NoError(t, err) + + location := file.NewLocation("garbage.bin") + require.NotPanics(t, func() { + pkgs, rels := fetchPkgs(reader, location) + assert.Empty(t, pkgs) + assert.Empty(t, rels) + }) +} diff --git a/syft/pkg/cataloger/java/graalvm_native_image_pe_test.go b/syft/pkg/cataloger/java/graalvm_native_image_pe_test.go new file mode 100644 index 000000000..dbe4ca989 --- /dev/null +++ b/syft/pkg/cataloger/java/graalvm_native_image_pe_test.go @@ -0,0 +1,398 @@ +package java + +import ( + "bytes" + "debug/pe" + "encoding/binary" + "io" + "math" + "testing" + "time" + "unsafe" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + intFile "github.com/anchore/syft/internal/file" + "github.com/anchore/syft/syft/artifact" + "github.com/anchore/syft/syft/pkg" +) + +// pins the bound at the byte where an attribute ends flush with the directory: `j+sz > n` (not `>=`) is +// what lets attribute 3 be read out of a 36-byte exports buffer, since bytes 32:36 are entirely present. +func TestFetchExportAttribute(t *testing.T) { + // the four attributes fetchExportContent reads start right after this header + require.EqualValues(t, 20, unsafe.Sizeof(exportPrefixPE{})) + + tests := []struct { + name string + exports []byte + index int + wantValue uint32 + wantErr bool + }{ + { + name: "one byte short of attribute 3 is rejected", + exports: make([]byte, 35), + index: 3, + wantErr: true, + }, + { + name: "attribute 3 flush with the end of exports is read", + exports: func() []byte { + b := make([]byte, 36) + binary.LittleEndian.PutUint32(b[32:36], 0xDEADBEEF) + return b + }(), + index: 3, + wantValue: 0xDEADBEEF, + }, + { + name: "attribute 0 reads the first attribute slot", + exports: func() []byte { + b := make([]byte, 24) + binary.LittleEndian.PutUint32(b[20:24], 42) + return b + }(), + index: 0, + wantValue: 42, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ni := nativeImagePE{exports: tt.exports} + got, err := ni.fetchExportAttribute(tt.index) + if tt.wantErr { + require.ErrorContains(t, err, nativeImageInvalidIndexError) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantValue, got) + }) + } +} + +// pins the uint32->uint64 widening: computing `functionsBase + i*sz` in uint32 wraps a huge base back +// into range instead of erroring. +func TestFetchExportFunctionPointer(t *testing.T) { + tests := []struct { + name string + exports []byte + functionsBase uint32 + index uint32 + wantValue uint32 + wantErr bool + }{ + { + name: "functionsBase near the uint32 max must not wrap back into range", + exports: make([]byte, 64), + functionsBase: 0xFFFFFFFF, + index: 1, + wantErr: true, + }, + { + name: "an in-range function pointer is read", + exports: func() []byte { + b := make([]byte, 64) + binary.LittleEndian.PutUint32(b[16:20], 0xCAFEBABE) + return b + }(), + functionsBase: 8, + index: 2, + wantValue: 0xCAFEBABE, + }, + { + name: "a pointer flush with the end of exports is read", + exports: func() []byte { + b := make([]byte, 64) + binary.LittleEndian.PutUint32(b[60:64], 0x11223344) + return b + }(), + functionsBase: 60, + index: 0, + wantValue: 0x11223344, + }, + { + name: "a pointer straddling the end of exports", + exports: make([]byte, 64), + functionsBase: 62, + index: 0, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ni := nativeImagePE{exports: tt.exports} + var got uint32 + var err error + require.NotPanics(t, func() { + got, err = ni.fetchExportFunctionPointer(tt.functionsBase, tt.index) + }) + if tt.wantErr { + require.ErrorContains(t, err, nativeImageInvalidIndexError) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantValue, got) + }) + } +} + +// addressOfNames below the directory's VirtualAddress. VirtualAddress is deliberately huge so the +// underflow lands back in range; a small one would let an unrelated bound mask this guard's absence. +func TestFetchSbomSymbols_NameArrayPrecedingDirectoryGuard(t *testing.T) { + va := uint32(0xFFFFFFF0) + addressOfNames := uint32(5) + wrapped := addressOfNames - va // what addressBase would be if the guard were missing + require.Less(t, wrapped, uint32(32), "the fixture only proves the guard's necessity if the wrap lands in range") + + exports := make([]byte, 32) + le := binary.LittleEndian + // a name pointer table here would only be reached without the guard + le.PutUint32(exports[wrapped:], va+10) // index 0: resolves to a non-matching name + le.PutUint32(exports[wrapped+4:], va+5) // index 1: resolves to "sbom" + copy(exports[10:], "zzzz\x00") + copy(exports[5:], nativeImageSbomSymbol+"\x00") + + ni := nativeImagePE{exports: exports, exportSymbols: pe.DataDirectory{VirtualAddress: va}} + content := &exportContentPE{addressOfNames: addressOfNames, numberOfNames: 2} + + require.NotPanics(t, func() { ni.fetchSbomSymbols(content) }) + assert.Zero(t, content.addressOfSbom, "addressOfNames below the directory must not be read at all") + assert.Zero(t, content.addressOfSbomLength) + assert.Zero(t, content.addressOfSvmVersion) +} + +// a name pointer entry resolving below the directory. Same reasoning as the addressOfNames guard above. +func TestFetchSbomSymbols_SymbolAddressPrecedingDirectoryGuard(t *testing.T) { + va := uint32(0xFFFFFFF0) + exports := make([]byte, 32) + le := binary.LittleEndian + le.PutUint32(exports[0:4], va+10) // index 0: resolves to a non-matching name + le.PutUint32(exports[4:8], 5) // index 1: a symbolAddress below the directory + + wrapped := uint32(5) - va // what symbolBase would be for index 1 if the guard were missing + require.Less(t, wrapped, uint32(32), "the fixture only proves the guard's necessity if the wrap lands in range") + copy(exports[10:], "zzzz\x00") + copy(exports[wrapped:], nativeImageSbomSymbol+"\x00") + + ni := nativeImagePE{exports: exports, exportSymbols: pe.DataDirectory{VirtualAddress: va}} + content := &exportContentPE{addressOfNames: va, numberOfNames: 2} + + require.NotPanics(t, func() { ni.fetchSbomSymbols(content) }) + assert.Zero(t, content.addressOfSbom, "a symbolAddress below the directory must not be read as a name") + assert.Zero(t, content.addressOfSbomLength) + assert.Zero(t, content.addressOfSvmVersion) +} + +// a symbolBase past the end of exports. Needs no wraparound: without the guard the slice is simply out +// of range and panics. +func TestFetchSbomSymbols_SymbolBaseOutOfRangeGuard(t *testing.T) { + const va = uint32(0x2000) + exports := make([]byte, 8) + binary.LittleEndian.PutUint32(exports[0:4], va+13) // symbolBase would be 13, past an 8-byte buffer + + ni := nativeImagePE{exports: exports, exportSymbols: pe.DataDirectory{VirtualAddress: va}} + content := &exportContentPE{addressOfNames: va, numberOfNames: 1} + + require.NotPanics(t, func() { ni.fetchSbomSymbols(content) }) + assert.Zero(t, content.addressOfSbom) + assert.Zero(t, content.addressOfSbomLength) + assert.Zero(t, content.addressOfSvmVersion) +} + +// addressBase past the end of exports. Every other fixture has addressBase == 0, so this is the only case +// reaching the `k > n` half: without it `n-k` underflows, clears the size check, and the slice panics. +func TestFetchSbomSymbols_AddressBaseOutOfRangeGuard(t *testing.T) { + const va = uint32(0x2000) + exports := make([]byte, 16) + + ni := nativeImagePE{exports: exports, exportSymbols: pe.DataDirectory{VirtualAddress: va}} + content := &exportContentPE{addressOfNames: 0x2064, numberOfNames: 1} // addressBase = 0x64 = 100, past exports + + require.NotPanics(t, func() { ni.fetchSbomSymbols(content) }) + assert.Zero(t, content.addressOfSbom) + assert.Zero(t, content.addressOfSbomLength) + assert.Zero(t, content.addressOfSvmVersion) +} + +// TestFetchSbomSymbols_FindsAllThreeSymbols is the happy path. fetchSbomSymbols records the loop index +// of a match, not its address. +func TestFetchSbomSymbols_FindsAllThreeSymbols(t *testing.T) { + const va = 0x2000 + exports := make([]byte, 96) + le := binary.LittleEndian + + // name pointer table: one uint32 RVA per name, in loop order. + // name pointer table: one uint32 RVA per name, in loop order. note fetchPkgs treats a stored index of 0 + // as "symbol not found", so no symbol may sit at index 0 or this would pass for the wrong reason + le.PutUint32(exports[0:4], va+52) // index 0: an unrelated name + le.PutUint32(exports[4:8], va+57) // index 1: "sbom" + le.PutUint32(exports[8:12], va+62) // index 2: "sbom_length" + le.PutUint32(exports[12:16], va+74) // index 3: "__svm_version_info" + + copy(exports[52:], "xxxx\x00") + copy(exports[57:], nativeImageSbomSymbol+"\x00") + copy(exports[62:], nativeImageSbomLengthSymbol+"\x00") + copy(exports[74:], nativeImageSbomVersionSymbol+"\x00") + + ni := nativeImagePE{exports: exports, exportSymbols: pe.DataDirectory{VirtualAddress: va}} + content := &exportContentPE{addressOfNames: va, numberOfNames: 4} + ni.fetchSbomSymbols(content) + + assert.EqualValues(t, 1, content.addressOfSbom) + assert.EqualValues(t, 2, content.addressOfSbomLength) + assert.EqualValues(t, 3, content.addressOfSvmVersion) +} + +// numberOfNames is read straight out of the file, so a hostile binary can claim ~4 billion names. The +// scan must stop when the name pointer table runs off the end of exports, not iterate that far. +func TestFetchSbomSymbols_HugeNameCountTerminatesQuickly(t *testing.T) { + exports := make([]byte, 16) // room for exactly 4 name pointer table entries + ni := nativeImagePE{exports: exports, exportSymbols: pe.DataDirectory{VirtualAddress: 0}} + content := &exportContentPE{addressOfNames: 0, numberOfNames: math.MaxUint32} + + start := time.Now() + require.NotPanics(t, func() { ni.fetchSbomSymbols(content) }) + assert.Less(t, time.Since(start), time.Second, + "the scan must stop at the end of exports, not loop numberOfNames times") +} + +// the one guard in fetchPkgs reachable without a real *pe.File: addressOfFunctions below the export +// directory base must error rather than underflow into a huge functionsBase. +func TestFetchPkgs_AddressOfFunctionsPrecedingDirectoryErrors(t *testing.T) { + const va = 0x2000 + exports := make([]byte, 96) + le := binary.LittleEndian + + // the four attributes fetchExportContent reads + le.PutUint32(exports[20:24], 10) // numberOfFunctions (unused before the guard under test) + le.PutUint32(exports[24:28], 4) // numberOfNames + le.PutUint32(exports[28:32], va-1) // addressOfFunctions: precedes the export directory + le.PutUint32(exports[32:36], va+36) + + // name pointer table at offset 36, resolving all three SBOM symbols so the guard under test is + // reached instead of the earlier "missing symbols" return + le.PutUint32(exports[36:40], va+52) + le.PutUint32(exports[40:44], va+57) + le.PutUint32(exports[44:48], va+62) + le.PutUint32(exports[48:52], va+74) + copy(exports[52:], "xxxx\x00") + copy(exports[57:], nativeImageSbomSymbol+"\x00") + copy(exports[62:], nativeImageSbomLengthSymbol+"\x00") + copy(exports[74:], nativeImageSbomVersionSymbol+"\x00") + + ni := nativeImagePE{exports: exports, exportSymbols: pe.DataDirectory{VirtualAddress: va}} + + var pkgs []pkg.Package + var rels []artifact.Relationship + var err error + require.NotPanics(t, func() { + pkgs, rels, err = ni.fetchPkgs() + }) + require.Error(t, err) + assert.NotContains(t, err.Error(), "recovered from panic", + "the guard must reject this cleanly, not rely on the recover") + assert.Contains(t, err.Error(), "exported function array precedes the export directory") + assert.Empty(t, pkgs) + assert.Empty(t, rels) +} + +// the three outcomes of weighing a declared size against both the cap and the bytes the file really has. +func TestReadExportDirectory(t *testing.T) { + t.Run("size over the cap is rejected", func(t *testing.T) { + dir := pe.DataDirectory{VirtualAddress: 0, Size: nativeImageMaxExportDirectorySize + 1} + _, err := readExportDirectory(bytes.NewReader(make([]byte, 1024)), dir) + require.Error(t, err) + assert.Contains(t, err.Error(), "byte limit") + }) + + t.Run("a size the file cannot satisfy is rejected", func(t *testing.T) { + dir := pe.DataDirectory{VirtualAddress: 0, Size: 600} + _, err := readExportDirectory(bytes.NewReader(make([]byte, 512)), dir) + require.Error(t, err) + assert.Contains(t, err.Error(), "remain") + }) + + t.Run("a size the file can exactly satisfy succeeds", func(t *testing.T) { + data := make([]byte, 200) + binary.LittleEndian.PutUint32(data[100:104], 0xABCDEF01) + dir := pe.DataDirectory{VirtualAddress: 100, Size: 100} // remaining bytes (200-100) exactly + + got, err := readExportDirectory(bytes.NewReader(data), dir) + require.NoError(t, err) + require.Len(t, got, 100) + assert.Equal(t, uint32(0xABCDEF01), binary.LittleEndian.Uint32(got[0:4])) + }) + + t.Run("a reader that cannot report its size is rejected", func(t *testing.T) { + data := make([]byte, 64) + r := struct{ io.ReaderAt }{bytes.NewReader(data)} // promotes ReadAt only, no Size() or Seek + dir := pe.DataDirectory{VirtualAddress: 0, Size: 32} + + _, err := readExportDirectory(r, dir) + require.ErrorContains(t, err, "unable to measure the binary") + }) +} + +// proves the remaining-bytes guard, not just the cap: a small, cheap file can still declare a size under +// the cap, and only the bytes really present can reject it before the read and its allocation. +func TestReadExportDirectory_DoesNotReadThePhantomBytes(t *testing.T) { + const realSize = 4096 + rec := &readSizeRecorder{Reader: bytes.NewReader(make([]byte, realSize))} + dir := pe.DataDirectory{VirtualAddress: 0, Size: nativeImageMaxExportDirectorySize} + + allocated := 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 + require.ErrorContains(t, err, "remain") + }) + + t.Logf("allocated %d bytes for a directory declaring %d against a %d byte file", allocated, dir.Size, realSize) + assert.Less(t, allocated, uint64(intFile.MB), + "rejecting an oversized declaration must not first allocate against it") + assert.LessOrEqual(t, rec.maxRead, realSize, + "no read should ask for more than the file actually holds") +} + +// shortReader has 64 addressable bytes but hands back fewer than asked with a nil error, the +// non-conforming shape syft/internal/unionreader documents for squashfs +type shortReader struct { + b []byte + perRead int +} + +func (s shortReader) Size() int64 { return int64(len(s.b)) } + +func (s shortReader) ReadAt(p []byte, off int64) (int, error) { + if len(p) == 1 { // intFile.ReaderSize probes the last byte, and that probe has to succeed + p[0] = s.b[off] + return 1, nil + } + return copy(p[:s.perRead], s.b[off:off+int64(s.perRead)]), nil +} + +// regression for the real defect: a ReadAt handing back fewer bytes than asked with a nil error used to +// be accepted, and the zeroed tail of the buffer parsed as export directory data. +func TestReadExportDirectory_RejectsShortReadWithNilError(t *testing.T) { + tests := []struct { + name string + perRead int + }{ + {name: "reader answers with zero bytes and a nil error", perRead: 0}, + {name: "reader answers with a partial read and a nil error", perRead: 4}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := shortReader{b: make([]byte, 64), perRead: tt.perRead} + dir := pe.DataDirectory{VirtualAddress: 0, Size: 32} + + _, err := readExportDirectory(r, dir) + require.ErrorContains(t, err, "read") + }) + } +}