STRLCPY/syft

fix: nil panic in graalvm cataloger (#1468)

* normalize error handling and recover from panics while parsing binaries
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
Co-authored-by: Alex Goodman <[email protected]>

Keith Zantow committed with GitHub 1 year ago

6cf668f7

1 parent 2ec4371c

Revision indexing in progress... (symbol navigation in revisions will be accurate after indexed)

Total 4 files Show one by one

■ ■ ■ ■ ■ ■

syft/pkg/cataloger/java/graalvm_native_image_cataloger.go

		skipped 114 lines
115	115		func getPackage(component nativeImageComponent) pkg.Package {
116	116		var cpes []cpe.CPE
117	117		for _, property := range component.Properties {
118		-	cpe, err := cpe.New(property.Value)
	118	+	c, err := cpe.New(property.Value)
119	119		if err != nil {
120	120		log.Debugf("native-image cataloger: could not parse CPE: %v.", err)
121	121		continue
122	122		}
123		-	cpes = append(cpes, cpe)
	123	+	cpes = append(cpes, c)
124	124		}
125	125		return pkg.Package{
126	126		Name: component.Name,
		skipped 12 lines
139	139		}
140	140
141	141		// decompressSbom returns the packages given within a native image executable's SBOM.
142		-	func decompressSbom(databuf []byte, sbomStart uint64, lengthStart uint64) ([]pkg.Package, error) {
	142	+	func decompressSbom(dataBuf []byte, sbomStart uint64, lengthStart uint64) ([]pkg.Package, error) {
143	143		var pkgs []pkg.Package
144	144
145	145		lengthEnd := lengthStart + 8
146		-	buflen := len(databuf)
147		-	if lengthEnd > uint64(buflen) {
148		-	return nil, errors.New("the sbom_length symbol overflows the binary")
	146	+	bufLen := len(dataBuf)
	147	+	if lengthEnd > uint64(bufLen) {
	148	+	return nil, errors.New("the 'sbom_length' symbol overflows the binary")
149	149		}
150	150
151		-	length := databuf[lengthStart:lengthEnd]
	151	+	length := dataBuf[lengthStart:lengthEnd]
152	152		p := bytes.NewBuffer(length)
153	153		var storedLength uint64
154	154		err := binary.Read(p, binary.LittleEndian, &storedLength)
155	155		if err != nil {
156		-	log.Debugf("native-image-cataloger: could not read from binary file.")
157		-	return nil, err
	156	+	return nil, fmt.Errorf("could not read from binary file: %w", err)
158	157		}
159		-	log.Tracef("native-image cataloger: found SBOM of length %d.", storedLength)
	158	+
	159	+	log.WithFields("len", storedLength).Trace("native-image cataloger: found SBOM")
160	160		sbomEnd := sbomStart + storedLength
161		-	if sbomEnd > uint64(buflen) {
	161	+	if sbomEnd > uint64(bufLen) {
162	162		return nil, errors.New("the sbom symbol overflows the binary")
163	163		}
164		-	sbomCompressed := databuf[sbomStart:sbomEnd]
	164	+
	165	+	sbomCompressed := dataBuf[sbomStart:sbomEnd]
165	166		p = bytes.NewBuffer(sbomCompressed)
166	167		gzreader, err := gzip.NewReader(p)
167	168		if err != nil {
168		-	log.Debugf("native-image cataloger: could not decompress the SBOM.")
169		-	return nil, err
	169	+	return nil, fmt.Errorf("could not decompress the native-image SBOM: %w", err)
170	170		}
	171	+
171	172		output, err := io.ReadAll(gzreader)
172	173		if err != nil {
173		-	log.Debugf("native-image cataloger: could not read the decompressed SBOM.")
174		-	return nil, err
	174	+	return nil, fmt.Errorf("could not read the native-image SBOM: %w", err)
175	175		}
	176	+
176	177		var sbomContent nativeImageCycloneDX
177	178		err = json.Unmarshal(output, &sbomContent)
178	179		if err != nil {
179		-	log.Debugf("native-image cataloger: could not unmarshal JSON.")
180		-	return nil, err
	180	+	return nil, fmt.Errorf("could not unmarshal the native-image SBOM: %w", err)
181	181		}
182	182
183	183		for _, component := range sbomContent.Components {
184	184		p := getPackage(component)
185	185		pkgs = append(pkgs, p)
186	186		}
	187	+
187	188		return pkgs, nil
188	189		}
189	190
190	191		// fileError logs an error message when an executable cannot be read.
191	192		func fileError(filename string, err error) (nativeImage, error) {
192	193		// We could not read the file as a binary for the desired platform, but it may still be a native-image executable.
193		-	log.Debugf("native-image cataloger: unable to read executable (file=%q): %v.", filename, err)
194		-	return nil, err
	194	+	return nil, fmt.Errorf("unable to read executable (file=%q): %w", filename, err)
195	195		}
196	196
197	197		// newElf reads a Native Image from an ELF executable.
		skipped 40 lines
238	238		case *pe.OptionalHeader64:
239	239		exportSymbolsDataDirectory = h.DataDirectory[0]
240	240		default:
241		-	return nil, fmt.Errorf("unable to get exportSymbolsDataDirectory from binary: %s", filename)
	241	+	return nil, fmt.Errorf("unable to get 'exportSymbolsDataDirectory' from binary: %s", filename)
242	242		}
243	243		// If we have no exported symbols it is not a Native Image
244	244		if exportSymbolsDataDirectory.Size == 0 {
		skipped 3 lines
248	248		exports := make([]byte, exportSymbolsDataDirectory.Size)
249	249		_, err = r.ReadAt(exports, int64(exportSymbolsOffset))
250	250		if err != nil {
251		-	log.Debugf("native-image cataloger: could not read the exported symbols data directory: %v.", err)
252		-	return fileError(filename, err)
	251	+	return fileError(filename, fmt.Errorf("could not read the exported symbols data directory: %w", err))
253	252		}
254	253		return nativeImagePE{
255	254		file: bi,
		skipped 17 lines
273	272		}
274	273
275	274		// fetchPkgs obtains the packages given in the binary.
276		-	func (ni nativeImageElf) fetchPkgs() ([]pkg.Package, error) {
	275	+	func (ni nativeImageElf) fetchPkgs() (pkgs []pkg.Package, retErr error) {
	276	+	defer func() {
	277	+	if r := recover(); r != nil {
	278	+	// this can happen in cases where a malformed binary is passed in can be initially parsed, but not
	279	+	// used without error later down the line.
	280	+	retErr = fmt.Errorf("recovered from panic: %v", r)
	281	+	}
	282	+	}()
	283	+
277	284		bi := ni.file
	285	+	if bi == nil {
	286	+	log.Debugf("native-image cataloger: file is nil")
	287	+	return nil, nil
	288	+	}
278	289		var sbom elf.Symbol
279	290		var sbomLength elf.Symbol
280	291		var svmVersion elf.Symbol
281	292
282	293		si, err := bi.Symbols()
283	294		if err != nil {
284		-	log.Debugf("native-image cataloger: no symbols found.")
285		-	return nil, err
	295	+	return nil, fmt.Errorf("no symbols found in binary: %w", err)
	296	+	}
	297	+	if si == nil {
	298	+	return nil, errors.New(nativeImageMissingSymbolsError)
286	299		}
287	300		for _, s := range si {
288	301		switch s.Name {
		skipped 6 lines
295	308		}
296	309		}
297	310		if sbom.Value == 0 \|\| sbomLength.Value == 0 \|\| svmVersion.Value == 0 {
298		-	log.Debugf("native-image cataloger: %v", nativeImageMissingSymbolsError)
299	311		return nil, errors.New(nativeImageMissingSymbolsError)
300	312		}
301	313		dataSection := bi.Section(".data")
302	314		if dataSection == nil {
303		-	log.Debugf("native-image cataloger: .data section missing from ELF file.")
304		-	return nil, err
	315	+	return nil, fmt.Errorf("no .data section found in binary: %w", err)
305	316		}
306	317		dataSectionBase := dataSection.SectionHeader.Addr
307	318		data, err := dataSection.Data()
308	319		if err != nil {
309		-	log.Debugf("native-image cataloger: cannot read the .data section.")
310		-	return nil, err
	320	+	return nil, fmt.Errorf("cannot read the .data section: %w", err)
311	321		}
312	322		sbomLocation := sbom.Value - dataSectionBase
313	323		lengthLocation := sbomLength.Value - dataSectionBase
		skipped 2 lines
316	326		}
317	327
318	328		// fetchPkgs obtains the packages from a Native Image given as a Mach O file.
319		-	func (ni nativeImageMachO) fetchPkgs() ([]pkg.Package, error) {
	329	+	func (ni nativeImageMachO) fetchPkgs() (pkgs []pkg.Package, retErr error) {
	330	+	defer func() {
	331	+	if r := recover(); r != nil {
	332	+	// this can happen in cases where a malformed binary is passed in can be initially parsed, but not
	333	+	// used without error later down the line.
	334	+	retErr = fmt.Errorf("recovered from panic: %v", r)
	335	+	}
	336	+	}()
	337	+
320	338		var sbom macho.Symbol
321	339		var sbomLength macho.Symbol
322	340		var svmVersion macho.Symbol
323	341
324	342		bi := ni.file
	343	+	if bi == nil {
	344	+	log.Debugf("native-image cataloger: file is nil")
	345	+	return nil, nil
	346	+	}
	347	+	if bi.Symtab == nil {
	348	+	return nil, errors.New(nativeImageMissingSymbolsError)
	349	+	}
325	350		for _, s := range bi.Symtab.Syms {
326	351		switch s.Name {
327	352		case "_" + nativeImageSbomSymbol:
		skipped 5 lines
333	358		}
334	359		}
335	360		if sbom.Value == 0 \|\| sbomLength.Value == 0 \|\| svmVersion.Value == 0 {
336		-	log.Debugf("native-image cataloger: %v.", nativeImageMissingSymbolsError)
337	361		return nil, errors.New(nativeImageMissingSymbolsError)
338	362		}
339	363
		skipped 1 lines
341	365		if dataSegment == nil {
342	366		return nil, nil
343	367		}
344		-	databuf, err := dataSegment.Data()
	368	+	dataBuf, err := dataSegment.Data()
345	369		if err != nil {
346	370		log.Debugf("native-image cataloger: cannot obtain buffer from data segment.")
347	371		return nil, nil
		skipped 1 lines
349	373		sbomLocation := sbom.Value - dataSegment.Addr
350	374		lengthLocation := sbomLength.Value - dataSegment.Addr
351	375
352		-	return decompressSbom(databuf, sbomLocation, lengthLocation)
	376	+	return decompressSbom(dataBuf, sbomLocation, lengthLocation)
353	377		}
354	378
355	379		// fetchExportAttribute obtains an attribute from the exported symbols directory entry.
		skipped 40 lines
396	420		var err error
397	421		content.numberOfFunctions, err = ni.fetchExportAttribute(0)
398	422		if err != nil {
399		-	log.Debugf("native-image cataloger: could not find the number of exported functions attribute: %v", err)
400		-	return nil, err
	423	+	return nil, fmt.Errorf("could not find the number of exported 'number of functions' attribute: %w", err)
401	424		}
402	425		content.numberOfNames, err = ni.fetchExportAttribute(1)
403	426		if err != nil {
404		-	log.Debugf("native-image cataloger: could not find the number of exported names attribute: %v", err)
405		-	return nil, err
	427	+	return nil, fmt.Errorf("could not find the number of exported 'number of names' attribute: %w", err)
406	428		}
407	429		content.addressOfFunctions, err = ni.fetchExportAttribute(2)
408	430		if err != nil {
409		-	log.Debugf("native-image cataloger: could not find the exported functions attribute: %v", err)
410		-	return nil, err
	431	+	return nil, fmt.Errorf("could not find the exported 'address of functions' attribute: %w", err)
411	432		}
412	433		content.addressOfNames, err = ni.fetchExportAttribute(3)
413	434		if err != nil {
414		-	log.Debugf("native-image cataloger: could not find the exported names attribute: %v", err)
415		-	return nil, err
	435	+	return nil, fmt.Errorf("could not find the exported 'address of names' attribute: %w", err)
416	436		}
417	437		return content, nil
418	438		}
		skipped 41 lines
460	480		}
461	481
462	482		// fetchPkgs obtains the packages from a Native Image given as a PE file.
463		-	func (ni nativeImagePE) fetchPkgs() ([]pkg.Package, error) {
	483	+	func (ni nativeImagePE) fetchPkgs() (pkgs []pkg.Package, retErr error) {
	484	+	defer func() {
	485	+	if r := recover(); r != nil {
	486	+	// this can happen in cases where a malformed binary is passed in can be initially parsed, but not
	487	+	// used without error later down the line.
	488	+	retErr = fmt.Errorf("recovered from panic: %v", r)
	489	+	}
	490	+	}()
	491	+
464	492		content, err := ni.fetchExportContent()
465	493		if err != nil {
466	494		log.Debugf("native-image cataloger: could not fetch the content of the export directory entry: %v.", err)
		skipped 1 lines
468	496		}
469	497		ni.fetchSbomSymbols(content)
470	498		if content.addressOfSbom == uint32(0) \|\| content.addressOfSbomLength == uint32(0) \|\| content.addressOfSvmVersion == uint32(0) {
471		-	log.Debugf("native-image cataloger: %v.", nativeImageMissingSymbolsError)
472	499		return nil, errors.New(nativeImageMissingSymbolsError)
473	500		}
474	501		functionsBase := content.addressOfFunctions - ni.exportSymbols.VirtualAddress
475	502		sbomOffset := content.addressOfSbom
476	503		sbomAddress, err := ni.fetchExportFunctionPointer(functionsBase, sbomOffset)
477	504		if err != nil {
478		-	log.Debugf("native-image cataloger: cannot fetch SBOM pointer from exported functions: %v.", err)
479		-	return nil, err
	505	+	return nil, fmt.Errorf("could not fetch SBOM pointer from exported functions: %w", err)
480	506		}
481	507		sbomLengthOffset := content.addressOfSbomLength
482	508		sbomLengthAddress, err := ni.fetchExportFunctionPointer(functionsBase, sbomLengthOffset)
483	509		if err != nil {
484		-	log.Debugf("native-image cataloger: cannot fetch SBOM length pointer from exported functions: %v.", err)
485		-	return nil, err
	510	+	return nil, fmt.Errorf("could not fetch SBOM length pointer from exported functions: %w", err)
486	511		}
487	512		bi := ni.file
488	513		dataSection := bi.Section(".data")
489	514		if dataSection == nil {
490	515		return nil, nil
491	516		}
492		-	databuf, err := dataSection.Data()
	517	+	dataBuf, err := dataSection.Data()
493	518		if err != nil {
494	519		log.Debugf("native-image cataloger: cannot obtain buffer from .data section.")
495	520		return nil, nil
		skipped 1 lines
497	522		sbomLocation := sbomAddress - dataSection.VirtualAddress
498	523		lengthLocation := sbomLengthAddress - dataSection.VirtualAddress
499	524
500		-	return decompressSbom(databuf, uint64(sbomLocation), uint64(lengthLocation))
	525	+	return decompressSbom(dataBuf, uint64(sbomLocation), uint64(lengthLocation))
501	526		}
502	527
503	528		// fetchPkgs provides the packages available in a UnionReader.
		skipped 55 lines

■ ■ ■ ■ ■ ■

syft/pkg/cataloger/java/graalvm_native_image_cataloger_test.go

		skipped 20 lines
21	21		func TestParseNativeImage(t *testing.T) {
22	22		tests := []struct {
23	23		fixture string
	24	+	newFn func(filename string, r io.ReaderAt) (nativeImage, error)
24	25		}{
25	26		{
26		-	fixture: "test-fixtures/java-builds/packages/example-java-app",
	27	+	fixture: "example-java-app",
	28	+	newFn: newElf,
	29	+	},
	30	+	{
	31	+	fixture: "gcc-amd64-darwin-exec-debug",
	32	+	newFn: newMachO,
27	33		},
28	34		}
29	35		for _, test := range tests {
30		-	t.Run(path.Base(test.fixture), func(t *testing.T) {
31		-	f, err := os.Open(test.fixture)
	36	+	t.Run(test.fixture, func(t *testing.T) {
	37	+	f, err := os.Open("test-fixtures/java-builds/packages/" + test.fixture)
32	38		assert.NoError(t, err)
33	39		readerCloser := io.ReadCloser(ioutil.NopCloser(f))
34	40		reader, err := unionreader.GetUnionReader(readerCloser)
		skipped 1 lines
36	42		parsed := false
37	43		readers, err := unionreader.GetReaders(reader)
38	44		for _, r := range readers {
39		-	ni, err := newElf(test.fixture, r)
	45	+	ni, err := test.newFn(test.fixture, r)
40	46		assert.NoError(t, err)
41	47		_, err = ni.fetchPkgs()
42	48		if err == nil {
		skipped 65 lines
108	114		z := gzip.NewWriter(writebytes)
109	115		_, err = z.Write(sbom)
110	116		assert.NoError(t, err)
111		-	z.Close()
112		-	writebytes.Flush()
	117	+	_ = z.Close()
	118	+	_ = writebytes.Flush()
113	119		compressedsbom := b.Bytes()
114	120		sbomlength := uint64(len(compressedsbom))
115		-	binary.Write(writebytes, binary.LittleEndian, sbomlength)
116		-	writebytes.Flush()
	121	+	_ = binary.Write(writebytes, binary.LittleEndian, sbomlength)
	122	+	_ = writebytes.Flush()
117	123		compressedsbom = b.Bytes()
118	124		actual, err := decompressSbom(compressedsbom, 0, sbomlength)
119	125		assert.NoError(t, err)
		skipped 5 lines

■ ■ ■ ■ ■ ■

syft/pkg/cataloger/java/test-fixtures/java-builds/Makefile

		skipped 16 lines
17	17
18	18		archives: $(PKGSDIR)/example-java-app-maven-0.1.0.zip $(PKGSDIR)/example-java-app-maven-0.1.0.tar $(PKGSDIR)/example-java-app-maven-0.1.0.tar.gz
19	19
20		-	native-image: $(PKGSDIR)/example-java-app
	20	+	native-image: $(PKGSDIR)/example-java-app $(PKGSDIR)/gcc-amd64-darwin-exec-debug
21	21
22	22		# jars within archives...
23	23
		skipped 43 lines
67	67		# Native Image...
68	68		$(PKGSDIR)/example-java-app: $(PKGSDIR)/example-java-app-maven-0.1.0.jar
69	69		./build-example-java-app-native-image.sh $(PKGSDIR)
	70	+
	71	+	$(PKGSDIR)/gcc-amd64-darwin-exec-debug:
	72	+	./build-example-macho-binary.sh $(PKGSDIR)
70	73
71	74		# we need a way to determine if CI should bust the test cache based on the source material
72	75		$(PKGSDIR).fingerprint: clean-examples
		skipped 4 lines

■ ■ ■ ■ ■ ■

syft/pkg/cataloger/java/test-fixtures/java-builds/build-example-macho-binary.sh

1	+	#!/usr/bin/env bash
2	+	set -uxe
3	+
4	+	PKGSDIR=$1
5	+
6	+	curl https://raw.githubusercontent.com/blacktop/go-macho/master/internal/testdata/gcc-amd64-darwin-exec-debug.base64 \|
7	+	base64 -d > $PKGSDIR/gcc-amd64-darwin-exec-debug
8	+