Golang mod parser reports version that are not used in application

[jerbob92]

I'm currently looking into why the parser reports versions being used that are not even in my application.
I noticed it's because the issues were being reported in dependencies of dependencies (sometimes even further down the tree), the code was not reachable at all, so the code is not included in the application, yet it reports it as a vulnerability.

Now I do get this, it's impossible for the parser to see if a dependency is actually being used. The good thing is that Go automatically does this for you already. It does this by adding all dependencies into the go.mod file, even the transitive dependencies (marked with //indirect), as long as there is an import somewhere, it will be added to the go.mod file.

The Go blog says this about the go.sum file:

The go command uses the go.sum file to ensure that future downloads of these modules retrieve the same bits as the first download, to ensure the modules your project depends on do not change unexpectedly, whether for malicious, accidental, or other reasons.

So I don't really see why go.sum is used here, and not go.mod. Can we change the parser to use go.mod?
When you execute go mod edit -json you will get a JSON representation of the go.mod file that's very clean and easy to parse.

CC @rahul2393, since you have implemented this dep parser in #21

[rahul2393]

@knqyf263 Please help assign this to the person on rotation, let me know if you want me to look into this.

[knqyf263]

It would be appreciated if you look into it.

[jerbob92]

I have tried to change the implementation to go.mod in #76 using the golang.org/x/mod/modfile package.
It also handles the replace directive correctly now.

[jerbob92]

@rahul2393 @knqyf263 any chance you can take a look at the PR? This is causing a lot of false positives for us.

[jerbob92]

@rahul2393 @knqyf263 ping

[jerbob92]

@rahul2393 @knqyf263 ping

[jerbob92]

@rahul2393 @knqyf263 ping

[knqyf263]

Hi, we're sorry for the delay.

So I don't really see why go.sum is used here, and not go.mod. Can we change the parser to use go.mod?

AFAIK, it is supported from Go 1.17. We couldn't know the dependencies that are actually used in 1.16 or less. So, I think we should keep the existing go.sum parser and add go.mod parser in addition.

[jerbob92]

Hi, we're sorry for the delay.

So I don't really see why go.sum is used here, and not go.mod. Can we change the parser to use go.mod?

AFAIK, it is supported from Go 1.17. We couldn't know the dependencies that are actually used in 1.16 or less. So, I think we should keep the existing go.sum parser and add go.mod parser in addition.

Yeah, you're right, the transitively are added automatically starting with Go 1.17. However, we can't rely on go.sum for everything below 1.17, since the last version in there isn't necessarily the version that is being imported.

My proposal would be:

• Parse go.mod, read the go 1.xx directive
• If 1.17+, use the current logic
• If lower than 1.17, use the current logic for direct imports, and then append any missing dependencies from the go.sum file

That way we can have the least amount of false positives, and they would only happen in transitive dependencies.

[jerbob92]

@knqyf263 I tried to make a start with this, but the dep-parser is really focused on only reading 1 file, so it's hard to access both the go.mod and go.sum at the same time when it reaches the Parse method. Any idea?

[knqyf263]

However, we can't rely on go.sum for everything below 1.17, since the last version in there isn't necessarily the version that is being imported.

Could you elaborate on it? According to the reference, the newer version is preferred.

At the end of the traversal, the highest required versions comprise the build list
https://go.dev/ref/mod#minimal-version-selection

Do you mean specified versions in direct dependencies are prioritized? In the above example, even if E1.1 imports A1.3, A1.2 is preferred? In that case, go.sum contains A1.2 and A1.3, but A1.2 will be selected, right?

[knqyf263]

I tried to make a start with this, but the dep-parser is really focused on only reading 1 file, so it's hard to access both the go.mod and go.sum at the same time when it reaches the Parse method. Any idea?

We're trying to allow fanal to process multiple files at the same time, but as a workaround until then, we can let go-dep-parser parse both files respectively, then merge go.mod and go.sum in hook.

[jerbob92]

Do you mean specified versions in direct dependencies are prioritized? In the above example, even if E1.1 imports A1.3, A1.2 is preferred? In that case, go.sum contains A1.2 and A1.3, but A1.2 will be selected, right?

Yeah, that's what I was thinking.
However, I just tried it, It looks like go mod tidy always changes the version in the main go.mod to the most recent version in the transitive dependencies, even if you try to set it to a lower version in your main go.mod file.

So let's say I import github.com/google/uuid version v1.3.0 in a dependency, and then v1.2.0 in my main package, go mod tidy will always rewrite it to v1.3.0 in the main go.mod. The only case this does not work that way, if you have a replace directive to force it to another version, but I have already solved that in my MR to "catch" that case.

[knqyf263]

Thanks for investigating! So, can we simply skip go.mod in Go 1.16 or less and take the latest versions in go.sum?

[jerbob92]

Thanks for investigating! So, can we simply skip go.mod in Go 1.16 or less and take the latest versions in go.sum?

I'd say it would still be good to use the go.mod as base for the dependency versions, so that it handles the replace directives correctly, and then merge in the go.sum for the transitive dependencies.

We're trying to allow fanal to process multiple files at the same time, but as a workaround until then, we can let go-dep-parser parse both files respectively, then merge go.mod and go.sum in hook.

That sounds doable. I think there is one problem though.
When Parse method gets the go.sum, it doesn't know which Go version is in the go.mod file, so it will then always return all dependencies that are in the go.sum file.
I think there is also no way to pass the Go version to the hook so that we can ignore the go.sum entries in case of Go 1.17+.
So then we would still get false positives at it will still merge in go.sum entries of transitive dependencies that are not in use.

[knqyf263]

I'd say it would still be good to use the go.mod as base for the dependency versions, so that it handles the replace directives correctly, and then merge in the go.sum for the transitive dependencies.

OK, it makes sense.

When Parse method gets the go.sum, it doesn't know which Go version is in the go.mod file, so it will then always return all dependencies that are in the go.sum file.

What if we add Indirect in Library? If go.mod has indirect dependencies in hook, we can assume it is Go 1.17+ and ignore go.sum dependencies. It may be a bit tricky, but we've considered that we should have Indirect field regardless as it is useful in other languages as well.

Here is PoC: 76e54d4

If it looks good, I'll open another PR adding Indirect.

[jerbob92]

What if we add Indirect in Library? If go.mod has indirect dependencies in hook, we can assume it is Go 1.17+ and ignore go.sum dependencies. It may be a bit tricky, but we've considered that we should have Indirect field regardless as it is useful in other languages as well.

Here is PoC: 76e54d4

If it looks good, I'll open another PR adding Indirect.

Yeah, sounds/looks good to me :)

[knqyf263]

Thanks for confirming. We'll add it.