From efa428270bb8183c2a184e7490894fa339c6a93a Mon Sep 17 00:00:00 2001
From: Aaron Lehmann <aaron.lehmann@blueowl.xyz>
Date: Mon, 14 Jan 2019 11:17:16 -0800
Subject: [PATCH] Make set hashing robust to bitwise XOR cancellations

The way sets (maps, structs, slices with the "set" tag) are hashed
relies on an XOR to mix the hashes of their elements. This allows
multiple independent items to interfere with each other through XOR
operations. For example, consider a "set" slice with the following two
structs:

[
  {Key: "a", Value: "v1"},
  {Key: "a", Value: "v2"}
]

Changing both keys from "a" to "b" will not change the hash value of the
overall object, since in either case the two keys will cancel out under
XOR arithmetic.

This change fixes the problem by adding a step after every set of
unordered hash operations that "hardens" the result by applying another
hash. It will prevent components of one unordered hashing operation (for
example, `Key: "a"` above) from interacting with the same data
encountered later in a different context.

Note that this changes hashes produced by the package for given inputs.

Closes: #18
---
 hashstructure.go               | 34 ++++++++++++++++++++++++++++++++++
 hashstructure_examples_test.go |  2 +-
 hashstructure_test.go          |  2 +-
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/hashstructure.go b/hashstructure.go
index ea13a15..8b990c8 100644
--- a/hashstructure.go
+++ b/hashstructure.go
@@ -211,6 +211,8 @@ func (w *walker) visit(v reflect.Value, opts *visitOpts) (uint64, error) {
 			h = hashUpdateUnordered(h, fieldHash)
 		}
 
+		h = hashFinishUnordered(w.h, h)
+
 		return h, nil
 
 	case reflect.Struct:
@@ -286,6 +288,8 @@ func (w *walker) visit(v reflect.Value, opts *visitOpts) (uint64, error) {
 				fieldHash := hashUpdateOrdered(w.h, kh, vh)
 				h = hashUpdateUnordered(h, fieldHash)
 			}
+
+			h = hashFinishUnordered(w.h, h)
 		}
 
 		return h, nil
@@ -313,6 +317,10 @@ func (w *walker) visit(v reflect.Value, opts *visitOpts) (uint64, error) {
 			}
 		}
 
+		if set {
+			h = hashFinishUnordered(w.h, h)
+		}
+
 		return h, nil
 
 	case reflect.String:
@@ -349,6 +357,32 @@ func hashUpdateUnordered(a, b uint64) uint64 {
 	return a ^ b
 }
 
+// After mixing a group of unique hashes with hashUpdateUnordered, it's always
+// necessary to call hashFinishUnordered. Why? Because hashUpdateUnordered
+// is a simple XOR, and calling hashUpdateUnordered on hashes produced by
+// hashUpdateUnordered can effectively cancel out a previous change to the hash
+// result if the same hash value appears later on. For example, consider:
+//
+//   hashUpdateUnordered(hashUpdateUnordered("A", "B"), hashUpdateUnordered("A", "C")) =
+//   H("A") ^ H("B")) ^ (H("A") ^ H("C")) =
+//   (H("A") ^ H("A")) ^ (H("B") ^ H(C)) =
+//   H(B) ^ H(C) =
+//   hashUpdateUnordered(hashUpdateUnordered("Z", "B"), hashUpdateUnordered("Z", "C"))
+//
+// hashFinishUnordered "hardens" the result, so that encountering partially
+// overlapping input data later on in a different context won't cancel out.
+func hashFinishUnordered(h hash.Hash64, a uint64) uint64 {
+	h.Reset()
+
+	// We just panic if the writes fail
+	e1 := binary.Write(h, binary.LittleEndian, a)
+	if e1 != nil {
+		panic(e1)
+	}
+
+	return h.Sum64()
+}
+
 // visitFlag is used as a bitmask for affecting visit behavior
 type visitFlag uint
 
diff --git a/hashstructure_examples_test.go b/hashstructure_examples_test.go
index 11d6efd..3ae925a 100644
--- a/hashstructure_examples_test.go
+++ b/hashstructure_examples_test.go
@@ -28,5 +28,5 @@ func ExampleHash() {
 
 	fmt.Printf("%d", hash)
 	// Output:
-	// 6691276962590150517
+	// 1839806922502695369
 }
diff --git a/hashstructure_test.go b/hashstructure_test.go
index 13ab834..3b764e4 100644
--- a/hashstructure_test.go
+++ b/hashstructure_test.go
@@ -97,7 +97,7 @@ func TestHash_equal(t *testing.T) {
 		{
 			struct{ Lname, Fname string }{"foo", "bar"},
 			struct{ Fname, Lname string }{"bar", "foo"},
-			true,
+			false,
 		},
 
 		{