go-fuzz icon indicating copy to clipboard operation
go-fuzz copied to clipboard

go-fuzz-dep: sonar serialization contains faulty type assumption

Open josharian opened this issue 2 years ago • 0 comments

While fuzzing something, I got this crash:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x8 pc=0x10210d414]

goroutine 1 [running]:
go-fuzz-dep.serialize({0x0, 0x0}, {0x102205300?, 0x140000aa900?}, {0x1400011db60?, 0xbdbfefbfbdef2f20?, 0x1400011db78?})
	go-fuzz-dep/sonar.go:150 +0xd94
go-fuzz-dep.Sonar({0x0, 0x0}, {0x102205300, 0x140000aa900}, 0x7c700)
	go-fuzz-dep/sonar.go:31 +0x54
reflect.DeepEqual.func2(...)
	/Users/josh/go/1.18/src/reflect/deepequal.go:230
reflect.DeepEqual({0x0, 0x0}, {0x102205300?, 0x140000aa900})
	/Users/josh/go/1.18/src/reflect/deepequal.go:230 +0xcc
[...]

The relevant reflect.DeepEqual code is:

func DeepEqual(x, y any) bool {
	if x == nil || y == nil {
		return x == y  // <------
	}

The relevant sonar.go line is:

func serialize(v, v2 interface{}, buf []byte) (n, flags uint8) {
	switch vv := v.(type) {
        // omit many cases
	default:
		// Special case: string literal is compared with a variable of
		// user type with string underlying type:
		//	type Name string
		//	var name Name
		//	if name == "foo" { ... }
		if _, ok := v2.(string); ok {
			s := *(*string)((*iface)(unsafe.Pointer(&v)).val)  // <--------
			if len(s) <= SonarMaxLen {
				return uint8(copy(buf[:], s)), SonarString
			}
		}
        // ...

I think this code contains a faulty assumption. If v has an interface type (as with reflect.DeepEqual), it can be compared with a string without itself being a string. The iface conversion is thus unsafe. I believe in this case it is due to a nil interface being compared with a string, causing a nil pointer dereference when reading the iface.val field.

If anyone cares (unlikely), this should be made safer, probably by using reflect.Type.

josharian avatar Jun 22 '22 05:06 josharian