Fix the "undefined: fs in fs.FileMode" issue

[disconnect3d]

Hey,

We have had an internship project in Trail of Bits to improve go-fuzz recently which was done by @vfsrfs.

We are aware of the ongoing official work on native fuzzing support but since we still rely on go-fuzz we went ahead to improve its pain points and so that's why we propose this PR. Feel free to drop it if you feel it is too much or you do not want to introduce any changes in go-fuzz.

Below I am pasting the description from a PR merged to our fork of go-fuzz (trail-of-forks#1).

Issue

This PR addresses the issue that is described in #325

Reproduction

When compiling the instrumented version of the code below, go-fuzz-build crashes with the error undefined: fs in fs.FileMode.

package homedir

import (
	"os"
	"fmt"
)

func HomeDir() {
	p := "text.txt"
	info, _ := os.Stat(p)

	if info.Mode().Perm()&(1<<(uint(7))) != 0 {
		fmt.Println("crash")
	}
}

Cause

Looking into the instrumented code, we can see that the expression

info.Mode().Perm()&(1<<(uint(7)))

is instrumented to

__gofuzz_v1 := fs.FileMode(info.Mode().Perm() & (1 << 7)).

At the first glance we realize that the type conversion to fs.FileMode is causing this crash.

The underlying issue is that the fs package is not imported and therefore it shouldn’t be referenced in the instrumented code. This bug first appeared in go1.16, since in that version a type alias in the os package was introduced that mapped os.FileMode to fs.FileMode (see https://go.dev/src/os/types.go?s=798:825#L18). So in fact, this type conversion should have been os.FileMode instead of fs.FileMode. However, when the code is loaded and parsed for instrumentation by /x/tool/packages, the type-checker already resolves the type alias, so that we only get the type that is included in fs instead of the one included in os.

Fix

To fix this issue, I propose to scan every type that is identified by the /x/tool/packages type-checker and to do a lookup of the package the where types are defined. If there is a type that is defined in a package which is not imported, the instrumentation will add the corresponding import statement. This ensures, that if there is an explicit type conversion using that type (which is the root cause of this bug), we will still be able to compile the instrumented code. The semantics of the instrumented code should still be the same after adding the import statement, because the package must have been imported by one of the imports of the analyzed code (e.g. os imports fs), so that the initialization of the package is still being made. Since there is no guarantee, that there will be an explicit type conversion using the newly imported package, we must make sure to remove the unused imports. For this purpose, we run goimports to clear every unused import and then proceed to compile the instrumented code.

[josharian]

@thepudds you up for reviewing this?

You can do the work that goimports does without exec'ing goimports. See golang.org/x/tools/imports.Process.