BUG Probe for SAST throws nil pointer exception for some Repos

[aunovis-heidrich]

Describe the bug
If I run the sastToolConfigured probe on the https://github.com/toml-rs/toml repository it throws a memory exception.

Reproduction steps
Steps to reproduce the behavior:

scorecard.exe --repo=https://github.com/toml-rs/toml --probes=sastToolConfigured

On my setup (git-bash, Windows 11) this produces a stack trace:

Starting probe [sastToolConfigured]
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x18 pc=0x19f9b4f]

goroutine 1 [running]:
github.com/ossf/scorecard/v5/checks/raw.sastToolInCheckRuns(0xc0004a86e0)
        github.com/ossf/scorecard/[email protected]/checks/raw/sast.go:128 +0x2ef
github.com/ossf/scorecard/v5/checks/raw.SAST(0xc0004a86e0)
        github.com/ossf/scorecard/[email protected]/checks/raw/sast.go:53 +0x69
github.com/ossf/scorecard/v5/pkg/scorecard.assignRawData({0x21d249e, 0x12?}, 0xc0004a86e0, 0xc000554708)
        github.com/ossf/scorecard/[email protected]/pkg/scorecard/scorecard_result.go:350 +0xf6
github.com/ossf/scorecard/v5/pkg/scorecard.populateRawResults(0xc0004a86e0, {0xc000502230, 0x1, 0x440b9e?}, 0xc000554708)
        github.com/ossf/scorecard/[email protected]/pkg/scorecard/scorecard_result.go:407 +0x2ad
github.com/ossf/scorecard/v5/pkg/scorecard.runEnabledProbes(0xc0004a86e0, {0xc000502230, 0x1, 0x9?}, 0xc000554708)
        github.com/ossf/scorecard/[email protected]/pkg/scorecard/scorecard.go:222 +0x45
github.com/ossf/scorecard/v5/pkg/scorecard.runScorecard({_, _}, {_, _}, {_, _}, _, _, {0xc000502230, 0x1, ...}, ...)
        github.com/ossf/scorecard/[email protected]/pkg/scorecard/scorecard.go:171 +0xb1b
github.com/ossf/scorecard/v5/pkg/scorecard.Run({_, _}, {_, _}, {_, _, _})
        github.com/ossf/scorecard/[email protected]/pkg/scorecard/scorecard.go:428 +0x98f
github.com/ossf/scorecard/v5/cmd.rootCmd(0xc000537c20)
        github.com/ossf/scorecard/[email protected]/cmd/root.go:161 +0x97f
github.com/ossf/scorecard/v5/cmd.New.func2(0xc0002da308?, {0x21d2552?, 0x4?, 0x21d24da?})
        github.com/ossf/scorecard/[email protected]/cmd/root.go:67 +0x17
github.com/spf13/cobra.(*Command).execute(0xc0002da308, {0xc0000c2190, 0x2, 0x3})
        github.com/spf13/[email protected]/command.go:985 +0xaaa
github.com/spf13/cobra.(*Command).ExecuteC(0xc0002da308)
        github.com/spf13/[email protected]/command.go:1117 +0x3ff
github.com/spf13/cobra.(*Command).Execute(0xc000537c20?)
        github.com/spf13/[email protected]/command.go:1041 +0x13
main.main()
        github.com/ossf/scorecard/[email protected]/main.go:27 +0x1d

Expected behavior
I have not yet investigated what exactly happens, so I am uncertain if the probe should fail or not. Regardless, I think that it should not throw a memory exception.

Additional context

$ ./scorecard.exe version
         __  ____     ____    ___    ____    _____    ____      _      ____    ____
        / / / ___|   / ___|  / _ \  |  _ \  | ____|  / ___|    / \    |  _ \  |  _ \
       / /  \___ \  | |     | | | | | |_) | |  _|   | |       / _ \   | |_) | | | | |
  _   / /    ___) | | |___  | |_| | |  _ <  | |___  | |___   / ___ \  |  _ <  | |_| |
 (_) /_/    |____/   \____|  \___/  |_| \_\ |_____|  \____| /_/   \_\ |_| \_\ |____/
./scorecard: OpenSSF Scorecard

GitVersion:    v5.1.1
GitCommit:     cd152cb6742c5b8f2f3d2b5193b41d9c50905198
GitTreeState:  clean
BuildDate:     1739675462
GoVersion:     go1.23.6
Compiler:      gc
Platform:      windows/amd64

I am curious about what happens, so if I find the time I will investigate further. If I see a reasonable chance for me to fix this, I will create a PR. I can make no guarantees at this point, though.

[spencerschrock]

I am curious about what happens, so if I find the time I will investigate further

It has to do with our logger being nil here:

scorecard/checks/raw/sast.go

Lines 127 to 132 in 3b42b6e

	if sastTools[cr.App.Slug] {
	c.Dlogger.Debug(&checker.LogMessage{
	Path: cr.URL,
	Type: finding.FileTypeURL,
	Text: fmt.Sprintf("tool detected: %v", cr.App.Slug),
	})

Normally the logger is set when running checks,:

scorecard/checker/check_runner.go

Lines 89 to 99 in 3b42b6e

	// Run runs a given check.
	func (r *Runner) Run(ctx context.Context, c Check) CheckResult {
	// Sanity check.
	unsupported := ListUnsupported(r.CheckRequest.RequiredTypes, c.SupportedRequestTypes)
	if len(unsupported) != 0 {
	return CreateRuntimeErrorResult(r.CheckName,
	sce.WithMessage(sce.ErrUnsupportedCheck,
	fmt.Sprintf("requiredType: %s not supported by check %s", fmt.Sprint(unsupported), r.CheckName)))
	}
	l := NewLogger()

When running probes, that detail gets returned as a finding and doesn't need to be logged. So instead of changing runEnabledProbes to pass down a logger, perhaps it's easier to just add a nil check here before we try to log?

[aunovis-heidrich]

Is that a valid state, the logger being nil even though a check is run? If so, a nil check is the way to go in my opinion.

[spencerschrock]

Is that a valid state, the logger being nil even though a check is run? If so, a nil check is the way to go in my opinion.

Yes and no, during a normal check-bsaed run, we'd expect the logger to be set. But during a probe run, the logger isn't used. Since this is just a debug level statement, should be fine to do a nil check