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.
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.
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 (https://github.com/trailofbits/go-fuzz/pull/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 errorundefined: fs in fs.FileMode
.Cause
Looking into the instrumented code, we can see that the expression
is instrumented to
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 theos
package was introduced that mappedos.FileMode
tofs.FileMode
(see https://go.dev/src/os/types.go?s=798:825#L18). So in fact, this type conversion should have beenos.FileMode
instead offs.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 infs
instead of the one included inos
.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
importsfs
), 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.