fix: prevent CompactBlock.Proofs() cache poisoning on validation failure (backport #2847) (#2934)

Fixes
https://dashboard.hackenproof.com/manager/companies/celestia/celestia/reports/CELESTIA-216

## Summary

- `CompactBlock.Proofs()` populated `proofsCache` before verifying
Merkle root hashes. When verification failed, the non-nil cache caused
subsequent calls to return invalid proofs with a nil error, bypassing
the integrity check.
- Fix uses a temporary slice during computation and only commits to
`proofsCache` after both root hash validations pass.
- Adds `TestProofsCachePoisoning` covering original root mismatch,
parity root mismatch, and valid block (regression) cases.

## Test plan

- [x] `TestProofsCachePoisoning/original_root_mismatch_poisons_cache` —
verifies second call still returns error
- [x] `TestProofsCachePoisoning/parity_root_mismatch_poisons_cache` —
verifies parity path
- [x] `TestProofsCachePoisoning/valid_compact_block_still_works` —
regression for happy path + cache reuse
- [x] Full `consensus/propagation/types` test suite passes

🤖 Generated with [Claude Code](https://claude.com/claude-code)


---

<a
href="https://app.devin.ai/review/celestiaorg/celestia-core/pull/2847"
target="_blank">
  <picture>
<source media="(prefers-color-scheme: dark)"
srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1">
<img
src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1"
alt="Open with Devin">
  </picture>
</a>

<hr>This is an automatic backport of pull request #2847 done by
[Mergify](https://mergify.com).
<!-- devin-review-badge-begin -->

---

<a
href="https://app.devin.ai/review/celestiaorg/celestia-core/pull/2934"
target="_blank">
  <picture>
<source media="(prefers-color-scheme: dark)"
srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1">
<img
src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1"
alt="Open with Devin">
  </picture>
</a>
<!-- devin-review-badge-end -->

Co-authored-by: Rootul P <rootulp@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: 3 people

consensus/propagation/types/types.go

@@ -206,22 +206,23 @@ func (c *CompactBlock) Proofs() ([]*merkle.Proof, error) {
 		return nil, errors.New("invalid number of partset hashes")
 	}
 
-	c.proofsCache = make([]*merkle.Proof, 0, len(c.PartsHashes))
+	tempProofs := make([]*merkle.Proof, 0, len(c.PartsHashes))
 
 	root, proofs := merkle.ParallelProofsFromLeafHashes(c.PartsHashes[:total])
-	c.proofsCache = append(c.proofsCache, proofs...)
+	tempProofs = append(tempProofs, proofs...)
 
 	if !bytes.Equal(root, c.Proposal.BlockID.PartSetHeader.Hash) {
-		return c.proofsCache, fmt.Errorf("incorrect PartsHash: original root")
+		return nil, fmt.Errorf("incorrect PartsHash: original root")
 	}
 
 	parityRoot, eproofs := merkle.ParallelProofsFromLeafHashes(c.PartsHashes[total:])
-	c.proofsCache = append(c.proofsCache, eproofs...)
+	tempProofs = append(tempProofs, eproofs...)
 
 	if !bytes.Equal(c.BpHash, parityRoot) {
-		return c.proofsCache, fmt.Errorf("incorrect PartsHash: parity root")
+		return nil, fmt.Errorf("incorrect PartsHash: parity root")
 	}
 
+	c.proofsCache = tempProofs
 	return c.proofsCache, nil
 }
 

consensus/propagation/types/types_test.go

@@ -876,6 +876,113 @@ func TestRecoveryPart_ValidateBasic(t *testing.T) {
 	}
 }
 
+// TestProofsCachePoisoning verifies that Proofs() does not cache invalid proofs.
+// If the root hash verification fails on the first call, subsequent calls must
+// also return an error rather than serving the invalid proofs from cache.
+func TestProofsCachePoisoning(t *testing.T) {
+	const numParts = 4
+
+	// Generate valid leaf hashes and compute the correct Merkle roots.
+	originalHashes := make([][]byte, numParts)
+	parityHashes := make([][]byte, numParts)
+	for i := 0; i < numParts; i++ {
+		originalHashes[i] = rand.Bytes(tmhash.Size)
+		parityHashes[i] = rand.Bytes(tmhash.Size)
+	}
+
+	originalRoot, _ := merkle.ParallelProofsFromLeafHashes(originalHashes)
+	parityRoot, _ := merkle.ParallelProofsFromLeafHashes(parityHashes)
+
+	t.Run("original root mismatch poisons cache", func(t *testing.T) {
+		// Build PartsHashes with manipulated original hashes so the computed root won't match.
+		partsHashes := make([][]byte, numParts*ParityRatio)
+		for i := 0; i < numParts; i++ {
+			partsHashes[i] = rand.Bytes(tmhash.Size) // manipulated
+		}
+		copy(partsHashes[numParts:], parityHashes)
+
+		cb := &CompactBlock{
+			Proposal: types.Proposal{
+				BlockID: types.BlockID{
+					PartSetHeader: types.PartSetHeader{
+						Total: uint32(numParts),
+						Hash:  originalRoot, // correct root
+					},
+				},
+			},
+			BpHash:      parityRoot,
+			PartsHashes: partsHashes,
+		}
+
+		// First call: should fail because manipulated hashes don't match originalRoot.
+		_, err := cb.Proofs()
+		require.Error(t, err, "first call should return an error for root hash mismatch")
+
+		// Second call: must also fail. This is the bug — without the fix,
+		// proofsCache is non-nil so Proofs() returns (cache, nil).
+		_, err = cb.Proofs()
+		require.Error(t, err, "second call must also return an error; cache should not hide the failure")
+	})
+
+	t.Run("parity root mismatch poisons cache", func(t *testing.T) {
+		// Build PartsHashes with correct original hashes but manipulated parity hashes.
+		partsHashes := make([][]byte, numParts*ParityRatio)
+		copy(partsHashes, originalHashes)
+		for i := 0; i < numParts; i++ {
+			partsHashes[numParts+i] = rand.Bytes(tmhash.Size) // manipulated
+		}
+
+		cb := &CompactBlock{
+			Proposal: types.Proposal{
+				BlockID: types.BlockID{
+					PartSetHeader: types.PartSetHeader{
+						Total: uint32(numParts),
+						Hash:  originalRoot,
+					},
+				},
+			},
+			BpHash:      parityRoot,
+			PartsHashes: partsHashes,
+		}
+
+		// First call: original root matches, but parity root will not.
+		_, err := cb.Proofs()
+		require.Error(t, err, "first call should return an error for parity root mismatch")
+
+		// Second call: must also fail.
+		_, err = cb.Proofs()
+		require.Error(t, err, "second call must also return an error; cache should not hide the failure")
+	})
+
+	t.Run("valid compact block still works", func(t *testing.T) {
+		partsHashes := make([][]byte, numParts*ParityRatio)
+		copy(partsHashes, originalHashes)
+		copy(partsHashes[numParts:], parityHashes)
+
+		cb := &CompactBlock{
+			Proposal: types.Proposal{
+				BlockID: types.BlockID{
+					PartSetHeader: types.PartSetHeader{
+						Total: uint32(numParts),
+						Hash:  originalRoot,
+					},
+				},
+			},
+			BpHash:      parityRoot,
+			PartsHashes: partsHashes,
+		}
+
+		proofs, err := cb.Proofs()
+		require.NoError(t, err)
+		require.Len(t, proofs, numParts*ParityRatio)
+
+		// Second call should also succeed from cache.
+		proofs2, err := cb.Proofs()
+		require.NoError(t, err)
+		require.Equal(t, proofs, proofs2)
+	})
+}
+
 func makeBlockID(hash []byte, partSetSize uint32, partSetHash []byte) types.BlockID {
 	var (
 		h   = make([]byte, tmhash.Size)