x/vulndb: govulncheck surfaces function calls with trusted input (e.g., GO-2022-1039)

Report ID

GO-2022-1039

Suggestion/Comment

The content of the warning is:

calls regexp.MustCompile, which eventually calls regexp/syntax.Parse

My code does indeed call MustCompile, but all of the non-test call sites pass in a literal string to the regex. As such, the call is trusted. If I undestand this vulnerability report correctly, it effectively means we can't ever use regexes and get a clean bill of health from govulncheck.

package main

import (
	"fmt"
	"regexp"
)

var re = regexp.MustCompile("trusted")

func main() {
	fmt.Println(re.String())
}

I was able to confirm that upgrading to both go1.18.7 and go1.19.2 fixes this.

[tatianab]

Hi Yuval, thanks for filing an issue! You're absolutely correct that govulncheck will always surface this vulnerability (unless you update to the latest Go version, as you just did), whether the call is safe or not.

This is a tricky problem because there are so many ways that a certain call could be safe or unsafe, some of which are automatically detectable and some of which are not. So, for now, we mark calls as potentially vulnerable and leave it up to humans to decide if the vulnerability applies to their code.

I've transferred this issue to the main issue tracker, and the team will discuss this further. We're also open to ideas or suggestions.

[zpavlinovic]

As @tatianab pointed out, govulncheck will report this vulnerability whenever it concludes that regexp/syntax.Parse is called. It does not have a mechanism for figuring out if the call arguments are trusted.

Would some form of warning suppression help in your case? If so, would that be a code annotation, or a config file you'd be willing provide to govulncheck, or perhaps something entirely different?

Thanks for the quick reply, both!

Yes, warning suppression would be totally fine. I think the easiest from a user's perspective would be an annotation/comment directly in the code.

[zpavlinovic]

Thank you for your feedback! This will help us in planning for future iterations of govulncheck.

In case it's helpful (re "we mark calls as potentially vulnerable and leave it up to humans to decide", above): my use case is that I wanted to run govulncheck in a GitHub action for pull requests. So having a human look over the vulnerability initially is totally fine (expected, even), but having to do that each time effectively turns the action into a false-alarm generator that we'd quickly learn to ignore.

One option would be for me to get the results as json (which I know govulncheck supports), and then examine that to effectively do my own suppressions. Those would probably have to be at the file level, but it'd be better than nothing. But, having it built into the tool would be better still. :-)

Thanks again!