From c8c8d6c4039a476478dfcfb732ba83822b648288 Mon Sep 17 00:00:00 2001
From: Martin HS <martin@swende.se>
Date: Mon, 28 Apr 2025 08:37:02 +0200
Subject: [PATCH] trie: add edgecase for rangeproof correctness (#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.
---
 trie/proof.go      | 14 +++++++++++---
 trie/proof_test.go | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/trie/proof.go b/trie/proof.go
index 2e527348bf..751d6f620f 100644
--- a/trie/proof.go
+++ b/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")
diff --git a/trie/proof_test.go b/trie/proof_test.go
index fab3a97650..b3c9dd753c 100644
--- a/trie/proof_test.go
+++ b/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)
+	}
+}