Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG Probe for SAST throws nil pointer exception for some Repos #4531

Open
aunovis-heidrich opened this issue Feb 19, 2025 · 2 comments
Open
Labels
kind/bug Something isn't working

Comments

@aunovis-heidrich
Copy link
Contributor

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/v5@v5.1.1/checks/raw/sast.go:128 +0x2ef
github.com/ossf/scorecard/v5/checks/raw.SAST(0xc0004a86e0)
        github.com/ossf/scorecard/v5@v5.1.1/checks/raw/sast.go:53 +0x69
github.com/ossf/scorecard/v5/pkg/scorecard.assignRawData({0x21d249e, 0x12?}, 0xc0004a86e0, 0xc000554708)
        github.com/ossf/scorecard/v5@v5.1.1/pkg/scorecard/scorecard_result.go:350 +0xf6
github.com/ossf/scorecard/v5/pkg/scorecard.populateRawResults(0xc0004a86e0, {0xc000502230, 0x1, 0x440b9e?}, 0xc000554708)
        github.com/ossf/scorecard/v5@v5.1.1/pkg/scorecard/scorecard_result.go:407 +0x2ad
github.com/ossf/scorecard/v5/pkg/scorecard.runEnabledProbes(0xc0004a86e0, {0xc000502230, 0x1, 0x9?}, 0xc000554708)
        github.com/ossf/scorecard/v5@v5.1.1/pkg/scorecard/scorecard.go:222 +0x45
github.com/ossf/scorecard/v5/pkg/scorecard.runScorecard({_, _}, {_, _}, {_, _}, _, _, {0xc000502230, 0x1, ...}, ...)
        github.com/ossf/scorecard/v5@v5.1.1/pkg/scorecard/scorecard.go:171 +0xb1b
github.com/ossf/scorecard/v5/pkg/scorecard.Run({_, _}, {_, _}, {_, _, _})
        github.com/ossf/scorecard/v5@v5.1.1/pkg/scorecard/scorecard.go:428 +0x98f
github.com/ossf/scorecard/v5/cmd.rootCmd(0xc000537c20)
        github.com/ossf/scorecard/v5@v5.1.1/cmd/root.go:161 +0x97f
github.com/ossf/scorecard/v5/cmd.New.func2(0xc0002da308?, {0x21d2552?, 0x4?, 0x21d24da?})
        github.com/ossf/scorecard/v5@v5.1.1/cmd/root.go:67 +0x17
github.com/spf13/cobra.(*Command).execute(0xc0002da308, {0xc0000c2190, 0x2, 0x3})
        github.com/spf13/cobra@v1.8.1/command.go:985 +0xaaa
github.com/spf13/cobra.(*Command).ExecuteC(0xc0002da308)
        github.com/spf13/cobra@v1.8.1/command.go:1117 +0x3ff
github.com/spf13/cobra.(*Command).Execute(0xc000537c20?)
        github.com/spf13/cobra@v1.8.1/command.go:1041 +0x13
main.main()
        github.com/ossf/scorecard/v5@v5.1.1/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.

@aunovis-heidrich aunovis-heidrich added the kind/bug Something isn't working label Feb 19, 2025
@spencerschrock
Copy link
Member

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:

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,:

// 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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
Status: No status
Development

No branches or pull requests

2 participants