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

Jump to bottom

internal/fuzz: fuzzer appears to mutate "immutable" string inputs #48308

Closed
bcmills opened this issue Sep 10, 2021 · 6 comments
Closed

internal/fuzz: fuzzer appears to mutate "immutable" string inputs #48308

bcmills opened this issue Sep 10, 2021 · 6 comments
Labels
FrozenDueToAge fuzz Issues related to native fuzzing support NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone
Go1.18

Comments

@bcmills
Copy link
Contributor

bcmills commented Sep 10, 2021 •
edited

What version of Go are you using (go version)?

$ go version
go version devel go1.18-7c648e2ac Thu Sep 9 17:28:03 2021 +0000 linux/amd64

Does this issue reproduce with the latest release?

N/A

What operating system and processor architecture are you using (go env)?

go env Output
$ gotip env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/bcmills/.cache/go-build"
GOENV="/usr/local/google/home/bcmills/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/tmp/tmp.BeOImjohVZ/.gopath/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/tmp/tmp.BeOImjohVZ/.gopath"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/google/home/bcmills/sdk/gotip"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/google/home/bcmills/sdk/gotip/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel go1.18-7c648e2ac Thu Sep 9 17:28:03 2021 +0000"
GCCGO="/usr/local/google/home/bcmills/bin/gccgo"
AR="ar"
CC="gcc"
CXX="c++"
CGO_ENABLED="1"
GOMOD="/tmp/tmp.BeOImjohVZ/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3787708356=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Fuzz the following program:

package main

import (
	"sync"
	"testing"
)

const maxSize = 1 << 10

var (
	mu sync.Mutex
	// seen records a sample of strings seen by the program.
	// The sample is limited to maxSize entries.
	seen = map[string]bool{}
)

func FuzzMapAccess(f *testing.F) {
	f.Fuzz(func(t *testing.T, in string) {
		mu.Lock()
		defer mu.Unlock()

		for k := range seen {
			if !seen[k] {
				panic("map does not contain its own elements")
			}
		}

		if len(seen) >= maxSize {
			for arbitrary := range seen {
				delete(seen, arbitrary)
				break
			}
		}
		seen[in] = true
	})
}

What did you expect to see?

No crashes, but maybe some grumbling from the fuzzer because my fuzz function depends on shared state.

What did you see instead?

$ gotip test -fuzz=.
fuzzing, elapsed: 0.1s, execs: 48 (958/sec), workers: 12, interesting: 2
--- FAIL: FuzzMapAccess (0.05s)
        panic: map does not contain its own elements
        goroutine 58 [running]:
        runtime/debug.Stack()
                /usr/local/google/home/bcmills/sdk/gotip/src/runtime/debug/stack.go:24 +0x90
        testing.tRunner.func1.2({0x58cbe0, 0x5f0518})
                /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:1288 +0x265
        testing.tRunner.func1()
                /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:1295 +0x225
        panic({0x58cbe0, 0x5f0518})
                /usr/local/google/home/bcmills/sdk/gotip/src/runtime/panic.go:814 +0x207
        example.com/m.FuzzMapAccess.func1(0x0, {0xc000014540, 0x7})
                /tmp/tmp.kTefRYzwUx/main_test.go:24 +0x2af
        reflect.Value.call({0x58e740, 0x5c7c98, 0x13}, {0x5ba1c3, 0x4}, {0xc00007aa80, 0x2, 0x2})
                /usr/local/google/home/bcmills/sdk/gotip/src/reflect/value.go:542 +0x814
        reflect.Value.Call({0x58e740, 0x5c7c98, 0xc0065b4dd0}, {0xc00007aa80, 0x2, 0x2})
                /usr/local/google/home/bcmills/sdk/gotip/src/reflect/value.go:338 +0xc5
        testing.(*F).Fuzz.func1.1(0x0)
                /usr/local/google/home/bcmills/sdk/gotip/src/testing/fuzz.go:389 +0x1c6
        testing.tRunner(0xc0000836c0, 0xc0065a4680)
                /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:1342 +0x102
        created by testing.(*F).Fuzz.func1
                /usr/local/google/home/bcmills/sdk/gotip/src/testing/fuzz.go:378 +0x4e5

        --- FAIL: FuzzMapAccess (0.00s)

    Crash written to testdata/corpus/FuzzMapAccess/378a946ef8c0e68ab4b14a0131c7c7f2b1989e27b0868ac5869de94a2e1bda90
    To re-run:
    go test example.com/m -run=FuzzMapAccess/378a946ef8c0e68ab4b14a0131c7c7f2b1989e27b0868ac5869de94a2e1bda90
FAIL
exit status 1
FAIL    example.com/m   0.071s

CC @jayconrod @katiehockman
@bcmills
Copy link
Contributor Author

bcmills commented Sep 10, 2021

Or, a simpler reproducer (but one that isn't quite as "obviously correct", given that unsafeslice makes heavy use of unsafe 😅):

-- fuzz_race_test.go --
package main

import (
	"testing"

	"github.com/bcmills/unsafeslice"
)

func FuzzOfString(f *testing.F) {
	f.Fuzz(func(t *testing.T, in string) {
		_ = unsafeslice.OfString(in)
	})
}
-- go.mod --
module example.com/m

go 1.18

require github.com/bcmills/unsafeslice v0.2.0
-- go.sum --
github.com/bcmills/unsafeslice v0.2.0 h1:JJNFFTuXbXCpx2dMf4UM7b/NIeCaTbfVbkko1CetRt0=
github.com/bcmills/unsafeslice v0.2.0/go.mod h1:oGhXgjortna9x6yUECqcCh4PLKdy0cSRPI3zHnUyuSo=

$ gotip test -fuzz=. .
panic: mutation detected in string at address 0x00c000180000

goroutine 33 [running]:
github.com/bcmills/unsafeslice.(*mutationChecker).recheck(0xc006886a40)
        /tmp/tmp.BeOImjohVZ/.gopath/pkg/mod/github.com/bcmills/unsafeslice@v0.2.0/safe.go:69 +0xda
found a crash, minimizing...
panic: mutation detected in string at address 0x00c000280000

goroutine 5 [running]:
github.com/bcmills/unsafeslice.(*mutationChecker).recheck(0xc00006ffe0)
        /tmp/tmp.BeOImjohVZ/.gopath/pkg/mod/github.com/bcmills/unsafeslice@v0.2.0/safe.go:69 +0xda
panic: mutation detected in string at address 0x00c000280000

goroutine 17 [running]:
github.com/bcmills/unsafeslice.(*mutationChecker).recheck(0xc0066b1000)
        /tmp/tmp.BeOImjohVZ/.gopath/pkg/mod/github.com/bcmills/unsafeslice@v0.2.0/safe.go:69 +0xda
tfuzzing, elapsed: 1.9s, execs: 24436 (12834/sec), workers: 12, interesting: 2
--- FAIL: FuzzOfString (1.90s)
    fuzzing process terminated unexpectedly: exit status 2
    Crash written to testdata/corpus/FuzzOfString/b81103222cee49f99723202dfe0ae4f13323b75ee25a65514cffb57298f0d0e1
    To re-run:
    go test example.com/m -run=FuzzOfString/b81103222cee49f99723202dfe0ae4f13323b75ee25a65514cffb57298f0d0e1
FAIL
exit status 1
FAIL    example.com/m   1.923s
FAIL

@bcmills bcmills changed the title [dev.fuzz] internal/fuzz: fuzzer appears to mutate "immutable" strings used as inputs [dev.fuzz] internal/fuzz: fuzzer appears to mutate "immutable" string inputs Sep 10, 2021
@seankhliao seankhliao added fuzz Issues related to native fuzzing support NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 10, 2021
@bcmills
Copy link
Contributor Author

bcmills commented Sep 10, 2021

I updated the first code sample to use an even simpler program containing only one map.

@katiehockman katiehockman added this to the Go1.18 milestone Sep 14, 2021
@rolandshoemaker
Copy link
Member

I suspect this is because when we return a string we create it by pointing at the underlying scratch buffer, which will eventually result in prior values being mutated. The simple fix is to just allocate new memory for the string.

@rolandshoemaker
Copy link
Member

In fact I suspect the byte slice mutators will have the exact same problem.

@gopherbot
Copy link

Change https://golang.org/cl/349989 mentions this issue: internal/fuzz: allocate memory for mutated slices

@rolandshoemaker rolandshoemaker self-assigned this Sep 15, 2021
@rsc rsc changed the title [dev.fuzz] internal/fuzz: fuzzer appears to mutate "immutable" string inputs internal/fuzz: fuzzer appears to mutate "immutable" string inputs Sep 21, 2021
@gopherbot
Copy link

Change https://golang.org/cl/351312 mentions this issue: internal/fuzz: allocate memory for mutated strings

@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge fuzz Issues related to native fuzzing support NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
Go Security
Status: No status
Milestone
Go1.18
Development

No branches or pull requests
5 participants
@rolandshoemaker @bcmills @gopherbot @seankhliao @katiehockman