search

eBay / sbom-scorecard

Generate a score for your sbom to understand if it will actually be useful.
Apache License 2.0
221 stars 24 forks source link

fix nil pointer reference bug & NaN handling on invalid json input #32

Closed frenchi closed 1 year ago

frenchi commented 1 year ago

It is possible to cause a segfault by providing invalid json.

./sbom-scorecard score ../../examples/invalid.json                                                       ⏎ ✹ ✭
Guessed: cdx
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x11acab8]

goroutine 1 [running]:
github.com/ebay/sbom-scorecard/pkg/cdx.GetCycloneDXReport({0x7ff7bfeff647, 0x1b})
    .../sbom-scorecard/pkg/cdx/cdx_report.go:119 +0x2f8
github.com/ebay/sbom-scorecard/cmd/sbom-scorecard/cmd.glob..func1(0x13fef00?, {0xc000063930?, 0x1?, 0x1?})
    .../sbom-scorecard/cmd/sbom-scorecard/cmd/scorecard.go:70 +0x1c8
github.com/spf13/cobra.(*Command).execute(0x13fef00, {0xc0000638f0, 0x1, 0x1})
    .../github.com/spf13/cobra@v1.6.1/command.go:920 +0x847
github.com/spf13/cobra.(*Command).ExecuteC(0x13fec20)
    .../github.com/spf13/cobra@v1.6.1/command.go:1044 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
    .../github.com/spf13/cobra@v1.6.1/command.go:968
github.com/ebay/sbom-scorecard/cmd/sbom-scorecard/cmd.Execute()
    .../sbom-scorecard/cmd/sbom-scorecard/cmd/root.go:20 +0x25
main.main()
    .../sbom-scorecard/cmd/sbom-scorecard/main.go:8 +0x17

pkg/cdx/cdx_report.go#L127 Fix nil pointer reference in bom.Metadata.Tools which causes nil pointer segfault when an invalid json file is provided.

pkg/cdx/cdx_report.go#L65 Fix NaN handling leading to invalid score results when an invalid json file is provided.

The root of this issue is likely that upstream cyclonedx-go/blob/master/decode.go#L43 uses json.Decode (which accepts a stream) instead of json.Marshal.

nit: ioutil package deprecated: As of Go 1.16, this function simply calls os.ReadFile.

frenchi commented 1 year ago

So I believe the tests are failing because the underlying logic for cdx_report.go#L49 IsSpecCompliant() is erroneous, as opposed to the test being incorrect.

This is because of an issue in cdx_report.go#L105 GetCycloneDXReport(), decoder.Decode(bom) will never throw err.

justinabrahms commented 1 year ago

Okay. So this no longer segfaults, but it does give weird results. There's an assumption that cyclonedx would return an error if given invalid json. It didn't. I've just made https://github.com/CycloneDX/cyclonedx-go/pull/84 which addresses that upstream. That should cause it to fail with a 0 for a score when they release & we update our dependency.

So sorry for the delay in addressing this.