util/deephash: fix collisions between different types

bradfitz · bradfitz · commit 8c5c87be2608 · 2022-06-21T22:29:04.000-07:00
Updates tailscale#4883 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
diff --git a/util/deephash/deephash.go b/util/deephash/deephash.go
@@ -127,7 +127,18 @@ func Hash(v any) (s Sum) {
 	h.reset()
 	seedOnce.Do(initSeed)
 	h.hashUint64(seed)
-	h.hashValue(reflect.ValueOf(v), false)
+
+	rv := reflect.ValueOf(v)
+	if rv.IsValid() {
+		// Always treat the Hash input as an interface (it is), including hashing
+		// its type, otherwise two Hash calls of different types could hash to the
+		// same bytes off the different types and get equivalent Sum values. This is
+		// the same thing that we do for reflect.Kind Interface in hashValue, but
+		// the initial reflect.ValueOf from an interface value effectively strips
+		// the interface box off so we have to do it at the top level by hand.
+		h.hashType(rv.Type())
+		h.hashValue(rv, false)
+	}
 	return h.sum()
 }
 
diff --git a/util/deephash/deephash_test.go b/util/deephash/deephash_test.go
@@ -61,6 +61,7 @@ func TestHash(t *testing.T) {
 	}
 	type MyBool bool
 	type MyHeader tar.Header
+	var zeroFloat64 float64
 	tests := []struct {
 		in     tuple
 		wantEq bool
@@ -102,6 +103,10 @@ func TestHash(t *testing.T) {
 		{in: tuple{iface{&MyHeader{}}, iface{&tar.Header{}}}, wantEq: false},
 		{in: tuple{iface{[]map[string]MyBool{}}, iface{[]map[string]MyBool{}}}, wantEq: true},
 		{in: tuple{iface{[]map[string]bool{}}, iface{[]map[string]MyBool{}}}, wantEq: false},
+		{in: tuple{zeroFloat64, -zeroFloat64}, wantEq: false}, // Issue 4883 (false alarm)
+		{in: tuple{[]any(nil), 0.0}, wantEq: false},           // Issue 4883
+		{in: tuple{[]any(nil), uint8(0)}, wantEq: false},      // Issue 4883
+		{in: tuple{nil, nil}, wantEq: true},                   // Issue 4883
 		{
 			in: func() tuple {
 				i1 := 1
@@ -117,7 +122,7 @@ func TestHash(t *testing.T) {
 	for _, tt := range tests {
 		gotEq := Hash(tt.in[0]) == Hash(tt.in[1])
 		if gotEq != tt.wantEq {
-			t.Errorf("(Hash(%v) == Hash(%v)) = %v, want %v", tt.in[0], tt.in[1], gotEq, tt.wantEq)
+			t.Errorf("(Hash(%T %v) == Hash(%T %v)) = %v, want %v", tt.in[0], tt.in[0], tt.in[1], tt.in[1], gotEq, tt.wantEq)
 		}
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -61,6 +61,7 @@ func TestHash(t *testing.T) {`
`61`	`61`	`}`
`62`	`62`	`type MyBool bool`
`63`	`63`	`type MyHeader tar.Header`
	`64`	`+ var zeroFloat64 float64`
`64`	`65`	`tests := []struct {`
`65`	`66`	`in tuple`
`66`	`67`	`wantEq bool`
`@@ -102,6 +103,10 @@ func TestHash(t *testing.T) {`
`102`	`103`	`{in: tuple{iface{&MyHeader{}}, iface{&tar.Header{}}}, wantEq: false},`
`103`	`104`	`{in: tuple{iface{[]map[string]MyBool{}}, iface{[]map[string]MyBool{}}}, wantEq: true},`
`104`	`105`	`{in: tuple{iface{[]map[string]bool{}}, iface{[]map[string]MyBool{}}}, wantEq: false},`
	`106`	`+ {in: tuple{zeroFloat64, -zeroFloat64}, wantEq: false}, // Issue 4883 (false alarm)`
	`107`	`+ {in: tuple{[]any(nil), 0.0}, wantEq: false}, // Issue 4883`
	`108`	`+ {in: tuple{[]any(nil), uint8(0)}, wantEq: false}, // Issue 4883`
	`109`	`+ {in: tuple{nil, nil}, wantEq: true}, // Issue 4883`
`105`	`110`	`{`
`106`	`111`	`in: func() tuple {`
`107`	`112`	`i1 := 1`
`@@ -117,7 +122,7 @@ func TestHash(t *testing.T) {`
`117`	`122`	`for _, tt := range tests {`
`118`	`123`	`gotEq := Hash(tt.in[0]) == Hash(tt.in[1])`
`119`	`124`	`if gotEq != tt.wantEq {`
`120`		`- t.Errorf("(Hash(%v) == Hash(%v)) = %v, want %v", tt.in[0], tt.in[1], gotEq, tt.wantEq)`
	`125`	`+ t.Errorf("(Hash(%T %v) == Hash(%T %v)) = %v, want %v", tt.in[0], tt.in[0], tt.in[1], tt.in[1], gotEq, tt.wantEq)`
`121`	`126`	`}`
`122`	`127`	`}`
`123`	`128`	`}`