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

go-fuzz-build doesn't work with go modules (go 1.11.2) #195

Closed
sashka opened this issue Nov 22, 2018 · 26 comments · Fixed by #274
Closed

go-fuzz-build doesn't work with go modules (go 1.11.2) #195

sashka opened this issue Nov 22, 2018 · 26 comments · Fixed by #274

Comments

@sashka
Copy link

sashka commented Nov 22, 2018

FTR: I'm going to investigate this issue a bit. If the problem is on my side, I will close the issue.

Environment:

> go version
go version go1.11.2 darwin/amd64

> go env GOROOT GOPATH
/usr/local/Cellar/go/1.11.2/libexec
/Users/asd/go

Steps to reproduce inside GOPATH:

> pwd
/Users/asd/go/src/github.com/sashka/signedvalue

> go-fuzz-build github.com/sashka/signedvalue
failed to execute go build: exit status 1
go: finding github.com/dvyukov/go-fuzz v0.0.0-20181106053552-383a81f6d048
go: finding github.com/stephens2424/writerset v0.0.0-20150719204953-fe01f9c9e73f
go: finding github.com/elazarl/go-bindata-assetfs v1.0.0
go: finding github.com/sashka/signedvalue/go.fuzz.main latest
can't load package: package github.com/sashka/signedvalue/go.fuzz.main: unknown import path "github.com/sashka/signedvalue/go.fuzz.main": cannot find module providing package github.com/sashka/signedvalue/go.fuzz.main

Steps to reproduce outside GOPATH:

> cd /tmp
> git clone git@github.com:sashka/signedvalue
Cloning into 'signedvalue'...
...
> cd signedvalue/
> go-fuzz-build github.com/sashka/signedvalue
package dir 'go: finding github.com/dvyukov/go-fuzz/go-fuzz-dep latest
go: finding github.com/dvyukov/go-fuzz latest
/Users/asd/go/pkg/mod/github.com/dvyukov/go-fuzz@v0.0.0-20181106053552-383a81f6d048/go-fuzz-dep' does not end with import path 'github.com/dvyukov/go-fuzz/go-fuzz-dep'
@dvyukov
Copy link
Owner

dvyukov commented Nov 23, 2018

Please try inside GOPATH. Building outside of GOPATH was never tested nor intended to work.

@sashka
Copy link
Author

sashka commented Nov 23, 2018

OK, I'll dig into the first case.

@dvyukov
Copy link
Owner

dvyukov commented Nov 23, 2018

Another reason to remember golang/go#19109

@sashka sashka changed the title go-fuzz-build doesn't work under go 1.11.2 and modules go-fuzz-build doesn't work with go modules (go 1.11.2) Nov 23, 2018
@Crypt-iQ
Copy link

Any updates on this? I experienced similar problems while trying to fuzz another project. I tried running go-fuzz in the project directory itself and received the unknown import path error. I found that running go-fuzz in /Users/{user}/go worked for me. Not ideal because then the binary is not created in the project directory... but at least it works!

@dvyukov
Copy link
Owner

dvyukov commented Dec 19, 2018

I do not have any updates on this.

vanackere added a commit to trustelem/zxcvbn that referenced this issue Jan 24, 2019
@odeke-em
Copy link

Hello there, I've also just encountered this with

$ go-fuzz-build github.com/census-instrumentation/opencensus-service/fuzz/receivers/zipkin
package dir '/Users/emmanuelodeke/go/pkg/mod/github.com/dvyukov/go-fuzz@v0.0.0-20190215131527-76990782bc83/go-fuzz-dep' does not end with import path 'github.com/dvyukov/go-fuzz/go-fuzz-dep'

@dvyukov perhaps it might be useful to create releases for this repository or even initialize it with go-modules?

@odeke-em
Copy link

Kindly paging @bcmills :) #sorryForTheRandomPing

@thepudds
Copy link
Collaborator

See also #219

@thepudds
Copy link
Collaborator

thepudds commented Apr 4, 2019

Inlining Josh's comment from #219 (comment) now that #219 is closed as a dup:

I suspect (but have not tried) that all that is required to add module support is to (a) copy go.mod if present while copying packages and (b) synthesize a go.mod alongside our synthesized package main.

I intend to work on module support at some point, but it isn't high on my list at the moment. I suppose it should be higher. I'd be happy to review a PR.

In addition to that outline from Josh, I suspect:

  • the go.mod for the synthesized package main might need a replace directive back to the on-disk location of the module being fuzzed, such as replace github.com/some/fuzztarget => /tmp/blah/blah/fuzztarget or similar.
  • there also might be a need to set env variable GO111MODULE=on explicitly, or alternatively perhaps not set any GOPATH env variable for the temp directory?

However, I could also imagine being wrong about those things. ;-)

Some additional background on replace is in this FAQ from the modules wiki page. Some additional background on enabling module mode for the go command via GO111MODULE or similar is in this other FAQ.

For reference, I think this is where the package main is created and written in go-fuzz-build:

func (c *Context) createFuzzMain() string {
mainPkg := filepath.Join(c.fuzzpkg.PkgPath, "go.fuzz.main")
path := filepath.Join(c.workdir, "gopath", "src", mainPkg)
c.mkdirAll(path)
c.writeFile(filepath.Join(path, "main.go"), c.funcMain())
return mainPkg
}

josharian added a commit to josharian/go-fuzz that referenced this issue Apr 30, 2019
Updates dvyukov#195
Updates google/oss-fuzz/pull#2188
dvyukov pushed a commit that referenced this issue May 3, 2019
Updates #195
Updates google/oss-fuzz/pull#2188
josharian added a commit to josharian/go-fuzz that referenced this issue May 3, 2019
This is a follow-up to dvyukov#237.

Updates dvyukov#195
Updates google/oss-fuzz/pull#2188
dvyukov pushed a commit that referenced this issue May 4, 2019
This is a follow-up to #237.

Updates #195
Updates google/oss-fuzz/pull#2188
@pzl
Copy link

pzl commented May 17, 2019

for what it's worth, this worked fine with a mod-vendored project with minimal changes.

index 324fc7c..fa57092 100644
--- a/go-fuzz-build/main.go
+++ b/go-fuzz-build/main.go
@@ -58,7 +58,6 @@ func makeTags() string {
 // that clients can then modify and use for calls to go/packages.
 func basePackagesConfig() *packages.Config {
    cfg := new(packages.Config)
-   cfg.Env = append(os.Environ(), "GO111MODULE=off")
    return cfg
 }
 
@@ -234,7 +233,7 @@ func (c *Context) loadPkg(pkg string) {
    // See https://golang.org/issue/30826 and https://golang.org/issue/30828.
    rescfg := basePackagesConfig()
    rescfg.Mode = packages.NeedName
-   rescfg.BuildFlags = []string{"-tags", makeTags()}
+ rescfg.BuildFlags = []string{"-mod=vendor", "-tags", makeTags()}
    respkgs, err := packages.Load(rescfg, pkg)
    if err != nil {
        c.failf("could not resolve package %q: %v", pkg, err)
@@ -254,7 +253,7 @@ func (c *Context) loadPkg(pkg string) {
    // of invalid code than trying to compile instrumented code.
    cfg := basePackagesConfig()
    cfg.Mode = packages.LoadAllSyntax
-   cfg.BuildFlags = []string{"-tags", makeTags()}
+ cfg.BuildFlags = []string{"-mod=vendor", "-tags", makeTags()}
    // use custom ParseFile in order to get comments
    cfg.ParseFile = func(fset *token.FileSet, filename string, src []byte) (*ast.File, error) {
        return parser.ParseFile(fset, filename, src, parser.ParseComments)

obviously this is just force-setting things to use go -mod=vendor all the time, but as a POC, it works.

It might help to see how the golangci-lint project loads things? https://github.com/golangci/golangci-lint/blob/master/pkg/lint/load.go#L226

There is a config (file or CLI param) to set your preferred -mod= mode, if you're using modules, otherwise ignores and doesn't put -mod in the BuildFlags

@pzl
Copy link

pzl commented May 17, 2019

Hmm, it seems it actually may have pulled from $GOPATH/pkg/mod/<import> when performing the build.

It didn't use vendor/ entirely. Missed a few BuildFlag changes.

That being said, vendor or not, module support might be not painful to implement here.

reference:

$ go version
go version go1.12.4 linux/amd64

@sashka
Copy link
Author

sashka commented Jun 28, 2019

Looks like it works these days.
Closing the issue.

@sashka sashka closed this as completed Jun 28, 2019
@ririsoft
Copy link

go-fuzz-build still does not work with go modules.
Can you consider reopening the issue please ?

@sashka
Copy link
Author

sashka commented Jun 28, 2019

OK. Reopened.

@sashka sashka reopened this Jun 28, 2019
@yevgenypats
Copy link
Contributor

@dvyukov @josharian I would like to work (me or some one from my team) on this issue. But before jumping and implementing I wanted to understand how complicated this is and how it should be implemented in terms of the design? we can come up with a proposal as well but wanted to see if there is already an idea in mind?

@dvyukov
Copy link
Owner

dvyukov commented Aug 21, 2019

I can't help much. I don't know what modules are. On user interface level we try to mimic go tool as much as possible. On implementation level I don't have any concrete thoughts.
@josharian?

@yevgenypats
Copy link
Contributor

yevgenypats commented Aug 21, 2019

ccing also @mvdan as I know you did some hacky work arounds. maybe you have a suggestion in mind?

@mvdan
Copy link
Contributor

mvdan commented Aug 21, 2019

I'm pretty sure @josharian has a master plan here, I just don't know if he's made it public :)

Anyone is free to copy what I did here: https://github.com/mvdan/sh/tree/master/_fuzz/it

The steps are:

  • create a nested module that depends on the main module via a replace (see go.mod), and that contains fuzz.go
  • use a script to create a GOPATH with all necessary packages; see fuzz-gopath, this is possible with a few lines thanks to go mod vendor
  • run go-fuzz-build with the GOPATH set up above (and modules turned off); see fuzz-ci

As a bonus point, you can also track/pin the version of go-fuzz-build via go.mod; see tools.go. This system takes a bit of setup, but it works well for me.

@josharian
Copy link
Collaborator

I would like to work (me or some one from my team) on this issue.

Great!

But before jumping and implementing I wanted to understand how complicated this is and how it should be implemented in terms of the design? we can come up with a proposal as well but wanted to see if there is already an idea in mind?

A short proposal is a fine way to proceed. It is possible that to understand enough to write a proposal you will find you need to actually implement most/all of it. If that happens, it is also ok to put the proposal in the commit message and send a PR. :)

On user interface level we try to mimic go tool as much as possible. On implementation level I don't have any concrete thoughts.

I would hope that there would be no interface-level impact at all. go-fuzz-build should "just work" on both module-using and non-module-using packages.

I'm pretty sure @josharian has a master plan here, I just don't know if he's made it public :)

I'm afraid that my master plan was: "get in there and figure it out".

@bookmoons
Copy link

@mvdan Your pattern is working well for me. Thanks for posting it.

Do you know if it's possible to fuzz a private function in the target package? I'm trying to adapt some fuzz targets that hit private functions. Not sure if there's a way to get at them from the fuzz module.

@thepudds
Copy link
Collaborator

thepudds commented Sep 22, 2019

We'll see how things end up, but most likely google/oss-fuzz#2878 needs to land first before landing modules support in go-fuzz. (google/oss-fuzz#2878 is a hopefully a small change).

@yevgenypats
Copy link
Contributor

@thepudds any news with google/oss-fuzz#2878?

@yevgenypats
Copy link
Contributor

@thepudds maybe we can do some incremental addition of this feature. i.e by adding some flag that enable this support until google/oss-fuzz#2878 is solved?

@thepudds
Copy link
Collaborator

thepudds commented Oct 3, 2019

@thepudds any news with google/oss-fuzz#2878?

Hello @yevgenypats, there seems to be some loose consensus there, but it is still open. I just pinged one of the oss-fuzz owners. If you don't see progress there soon, you could add a friendly ping there if you want.

@yevgenypats
Copy link
Contributor

Hey @thepudds, sure, I can add a friendly ping. In the meantime I suggest #271 to remove pressure and give the users that are blocked by this feature to opt-in.

josharian added a commit to josharian/go-fuzz that referenced this issue Oct 3, 2019
yevgenypats pushed a commit to fuzzitdev/oss-fuzz that referenced this issue Oct 3, 2019
This will prevent breaking go-fuzz when it will support go modules
dvyukov/go-fuzz#195

and addressing this issue:
google#2878
Dor1s pushed a commit to google/oss-fuzz that referenced this issue Oct 3, 2019
This will prevent breaking go-fuzz when it will support go modules
dvyukov/go-fuzz#195

and addressing this issue:
#2878
@thepudds
Copy link
Collaborator

thepudds commented Oct 7, 2019

For anyone who is particularly eager to see modules support land, one good and hopefully quick way to help would be sending a PR for issue #252.

(That is not a gate on initial modules support, but it would help).

bradleyjkemp pushed a commit to bradleyjkemp/simple-fuzz that referenced this issue Oct 23, 2019
Updates dvyukov#195
Updates google/oss-fuzz/pull#2188
bradleyjkemp pushed a commit to bradleyjkemp/simple-fuzz that referenced this issue Oct 23, 2019
This is a follow-up to dvyukov#237.

Updates dvyukov#195
Updates google/oss-fuzz/pull#2188
jaqx0r added a commit to google/mtail that referenced this issue Dec 6, 2019
Still doesn't work because Prometheus client_golang broke everything for
non-modules in
github.com/prometheus/client_golang/commit/ee1078a03c073762e2660d305738bfdafbf69ba6

You can't resolve xxhash/v2 in the fuzzer bcause it doesn't support modules
yet, dvyukov/go-fuzz#195
jaqx0r added a commit to google/mtail that referenced this issue Dec 6, 2019
jaqx0r added a commit to google/mtail that referenced this issue Dec 16, 2019
…ind them.

Until dvyukov/go-fuzz#195 is fixed, we need to vendor
as some dependencies are only modules and cannot be found otherwise.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.