trie: add edgecase for rangeproof correctness (ethereum#31667)

This PR adds checking for an edgecase which theoretically can happen in
the range-prover. Right now, we check that a key does not overwrite a
previous one by checking that the key is increasing. However, if keys
are of different lengths, it is possible to create a key which is
increasing _and_ overwrites the previous key. Example: `0xaabbcc`
followed by `0xaabbccdd`.

This can not happen in go-ethereum, which always uses fixed-size paths
for accounts and storage slot paths in the trie, but it might happen if
the range prover is used without guaranteed fixed-size keys.

This PR also adds some testcases for the errors that are expected.

Author: holiman

trie/proof.go

@@ -485,10 +485,18 @@ func VerifyRangeProof(rootHash common.Hash, firstKey []byte, keys [][]byte, valu
 	if len(keys) != len(values) {
 		return false, fmt.Errorf("inconsistent proof data, keys: %d, values: %d", len(keys), len(values))
 	}
-	// Ensure the received batch is monotonic increasing and contains no deletions
+	// Ensure the received batch is
+	// - monotonically increasing,
+	// - not expanding down prefix-paths
+	// - and contains no deletions
 	for i := 0; i < len(keys); i++ {
-		if i < len(keys)-1 && bytes.Compare(keys[i], keys[i+1]) >= 0 {
-			return false, errors.New("range is not monotonically increasing")
+		if i < len(keys)-1 {
+			if bytes.Compare(keys[i], keys[i+1]) >= 0 {
+				return false, errors.New("range is not monotonically increasing")
+			}
+			if bytes.HasPrefix(keys[i+1], keys[i]) {
+				return false, errors.New("range contains path prefixes")
+			}
 		}
 		if len(values[i]) == 0 {
 			return false, errors.New("range contains deletion")

trie/proof_test.go

@@ -1000,3 +1000,35 @@ func TestRangeProofKeysWithSharedPrefix(t *testing.T) {
 		t.Error("expected more to be false")
 	}
 }
+
+// TestRangeProofErrors tests a few cases where the prover is supposed
+// to exit with errors
+func TestRangeProofErrors(t *testing.T) {
+	// Different number of keys to values
+	_, err := VerifyRangeProof((common.Hash{}), []byte{}, make([][]byte, 5), make([][]byte, 4), nil)
+	if have, want := err.Error(), "inconsistent proof data, keys: 5, values: 4"; have != want {
+		t.Fatalf("wrong error, have %q, want %q", err.Error(), want)
+	}
+	// Non-increasing paths
+	_, err = VerifyRangeProof((common.Hash{}), []byte{},
+		[][]byte{[]byte{2, 1}, []byte{2, 1}}, make([][]byte, 2), nil)
+	if have, want := err.Error(), "range is not monotonically increasing"; have != want {
+		t.Fatalf("wrong error, have %q, want %q", err.Error(), want)
+	}
+	// A prefixed path is never motivated. Inserting the second element will
+	// require rewriting/overwriting the previous value-node, thus can only
+	// happen if the data is corrupt.
+	_, err = VerifyRangeProof((common.Hash{}), []byte{},
+		[][]byte{[]byte{2, 1}, []byte{2, 1, 2}},
+		[][]byte{[]byte{1}, []byte{1}}, nil)
+	if have, want := err.Error(), "range contains path prefixes"; have != want {
+		t.Fatalf("wrong error, have %q, want %q", err.Error(), want)
+	}
+	// Empty values (deletions)
+	_, err = VerifyRangeProof((common.Hash{}), []byte{},
+		[][]byte{[]byte{2, 1}, []byte{2, 2}},
+		[][]byte{[]byte{1}, []byte{}}, nil)
+	if have, want := err.Error(), "range contains deletion"; have != want {
+		t.Fatalf("wrong error, have %q, want %q", err.Error(), want)
+	}
+}