From 57136945bda242fe0e08776e11f331a3c138e20f Mon Sep 17 00:00:00 2001
From: rene <41963722+renaynay@users.noreply.github.com>
Date: Fri, 11 Aug 2023 13:09:53 +0200
Subject: [PATCH] =?UTF-8?q?fix(header):=20Only=20bridge=20node=20should=20?=
 =?UTF-8?q?panic=20on=20data=20root=20mismatch=20in=20e=E2=80=A6=20(#2558)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This PR fixes a DoS first discovered by @Wondertan and then secondarily
by @vgonkivs 🤠

Only bridge nodes should panic on receiving a header where the computed
data root does not match the DataHash in the RawHeader on ExtendedHeader
validation.

Resolves #2555
---
 core/exchange.go    | 4 ++--
 core/header_test.go | 8 +++-----
 core/listener.go    | 2 +-
 header/header.go    | 4 ++--
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/core/exchange.go b/core/exchange.go
index e041ff980f..a9b3a532d5 100644
--- a/core/exchange.go
+++ b/core/exchange.go
@@ -115,7 +115,7 @@ func (ce *Exchange) Get(ctx context.Context, hash libhead.Hash) (*header.Extende
 	// construct extended header
 	eh, err := ce.construct(ctx, &block.Header, comm, vals, eds)
 	if err != nil {
-		return nil, fmt.Errorf("constructing extended header for height %d: %w", &block.Height, err)
+		panic(fmt.Errorf("constructing extended header for height %d: %w", &block.Height, err))
 	}
 	// verify hashes match
 	if !bytes.Equal(hash, eh.Hash()) {
@@ -155,7 +155,7 @@ func (ce *Exchange) getExtendedHeaderByHeight(ctx context.Context, height *int64
 	// create extended header
 	eh, err := ce.construct(ctx, &b.Header, &b.Commit, &b.ValidatorSet, eds)
 	if err != nil {
-		return nil, fmt.Errorf("constructing extended header for height %d: %w", b.Header.Height, err)
+		panic(fmt.Errorf("constructing extended header for height %d: %w", b.Header.Height, err))
 	}
 
 	ctx = ipld.CtxWithProofsAdder(ctx, adder)
diff --git a/core/header_test.go b/core/header_test.go
index 6315fbc143..1c89db9d6b 100644
--- a/core/header_test.go
+++ b/core/header_test.go
@@ -41,11 +41,9 @@ func TestMakeExtendedHeaderForEmptyBlock(t *testing.T) {
 
 func TestMismatchedDataHash_ComputedRoot(t *testing.T) {
 	header := headertest.RandExtendedHeader(t)
-
 	header.DataHash = rand.Bytes(32)
 
-	panicFn := func() {
-		header.Validate() //nolint:errcheck
-	}
-	assert.Panics(t, panicFn)
+	err := header.Validate()
+	assert.Contains(t, err.Error(), "mismatch between data hash commitment from"+
+		" core header and computed data root")
 }
diff --git a/core/listener.go b/core/listener.go
index 334aa9293a..17be12a780 100644
--- a/core/listener.go
+++ b/core/listener.go
@@ -160,7 +160,7 @@ func (cl *Listener) handleNewSignedBlock(ctx context.Context, b types.EventDataS
 	// generate extended header
 	eh, err := cl.construct(ctx, &b.Header, &b.Commit, &b.ValidatorSet, eds)
 	if err != nil {
-		return fmt.Errorf("making extended header: %w", err)
+		panic(fmt.Errorf("making extended header: %w", err))
 	}
 
 	// attempt to store block data if not empty
diff --git a/header/header.go b/header/header.go
index 0e386be8b5..d69b11d998 100644
--- a/header/header.go
+++ b/header/header.go
@@ -146,8 +146,8 @@ func (eh *ExtendedHeader) Validate() error {
 
 	// ensure data root from raw header matches computed root
 	if !bytes.Equal(eh.DAH.Hash(), eh.DataHash) {
-		panic(fmt.Sprintf("mismatch between data hash commitment from core header and computed data root "+
-			"at height %d: data hash: %X, computed root: %X", eh.Height(), eh.DataHash, eh.DAH.Hash()))
+		return fmt.Errorf("mismatch between data hash commitment from core header and computed data root "+
+			"at height %d: data hash: %X, computed root: %X", eh.Height(), eh.DataHash, eh.DAH.Hash())
 	}
 
 	// Make sure the header is consistent with the commit.