From 703063f71b3de4a1a9d5c24b2625be67e39a0f83 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Mon, 13 Apr 2026 07:29:35 -0700 Subject: [PATCH 1/2] fix: reject truncated proofs in validateCompleteness (GHSA-r9fq-g486-v8pg) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validateCompleteness's left-traversal loop exited on either `leafIndex == proof.Start()` or `len(nodes) == 0`. When the nodes slice was exhausted before leafIndex caught up, rightSubtrees was set to the (now empty) nodes slice, the right-side namespace check ran zero iterations, and the function silently returned nil — a completeness-gate bypass. The end-to-end VerifyLeafHashes / ComputeRootWithBasicValidation path was still protected by computeRoot, so the public API did not silently accept forged proofs. This is a defense-in-depth fix: any future caller that separates completeness checking from root verification would have been exposed. Add an explicit post-loop guard so early node exhaustion becomes an error, and cover both the empty-nodes and partially-consumed variants with unit tests. The existing right-side check is validated by a control test. Co-Authored-By: Claude Opus 4.6 (1M context) --- proof.go | 10 ++++ proof_completeness_test.go | 96 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 proof_completeness_test.go diff --git a/proof.go b/proof.go index 6f4ab14..b5ebe03 100644 --- a/proof.go +++ b/proof.go @@ -316,6 +316,16 @@ func (proof Proof) validateCompleteness(nth *NmtHasher, nID namespace.ID) error nodes = nodes[1:] leafIndex += uint64(subtreeSize) } + // If the nodes slice was exhausted before the left traversal reached + // proof.Start(), the proof is truncated. Without this guard, rightSubtrees + // below would be empty and the right-side namespace check would be + // silently skipped — see GHSA-r9fq-g486-v8pg. + if leafIndex != uint64(proof.Start()) { + return fmt.Errorf( + "proof nodes insufficient: left traversal reached leaf index %d, expected %d", + leafIndex, proof.Start(), + ) + } // rightSubtrees only contains the subtrees after r.End rightSubtrees := nodes diff --git a/proof_completeness_test.go b/proof_completeness_test.go new file mode 100644 index 0000000..b708d11 --- /dev/null +++ b/proof_completeness_test.go @@ -0,0 +1,96 @@ +package nmt + +import ( + "crypto/sha256" + "testing" + + "github.com/celestiaorg/nmt/namespace" + "github.com/stretchr/testify/require" +) + +// TestValidateCompleteness_TruncatedProof_RejectsEmptyNodes ensures that a +// proof whose nodes slice is empty (but whose start index is non-zero) is +// rejected. Before the fix, the left-traversal loop exited immediately on +// `len(nodes) > 0`, rightSubtrees became empty, and the function silently +// returned nil even though no completeness check was actually performed. +func TestValidateCompleteness_TruncatedProof_RejectsEmptyNodes(t *testing.T) { + nth := NewNmtHasher(sha256.New(), namespace.IDSize(1), false) + targetNID := namespace.ID{0x05} + + // start=8 requires the left traversal to walk indices [0, 8). With an + // empty nodes slice, the loop cannot consume anything and leafIndex + // stays at 0, never reaching proof.Start()=8. + proof := Proof{ + start: 8, + end: 9, + nodes: nil, + } + + err := proof.validateCompleteness(nth, targetNID) + require.Error(t, err, + "validateCompleteness must reject a proof whose nodes are "+ + "exhausted before the left traversal reaches proof.Start()") +} + +// TestValidateCompleteness_TruncatedProof_RejectsInsufficientNodes is the +// non-empty variant: the nodes slice has some entries but is still too short +// to carry the left traversal to proof.Start(). +func TestValidateCompleteness_TruncatedProof_RejectsInsufficientNodes(t *testing.T) { + nth := NewNmtHasher(sha256.New(), namespace.IDSize(1), false) + targetNID := namespace.ID{0x05} + + // A single well-formed NMT node whose namespace range is [0x01, 0x01] + // (strictly less than targetNID). As a leftSubtree this passes the + // left-side check; it's only in the slice so the left traversal + // consumes something before nodes are exhausted. + smallNID := []byte{0x01} + digest := sha256.Sum256([]byte("leftSubtreeNode")) + node := append(append([]byte{}, smallNID...), smallNID...) + node = append(node, digest[:]...) + + // For start=12: + // - iter 1: nextSubtreeSize(0, 12) = 8, consumes the only node, + // leafIndex advances to 8, nodes becomes empty. + // - iter 2: loop exits via len(nodes) > 0 == false, + // leafIndex=8 != proof.Start()=12. + proof := Proof{ + start: 12, + end: 13, + nodes: [][]byte{node}, + } + + err := proof.validateCompleteness(nth, targetNID) + require.Error(t, err, + "validateCompleteness must reject a proof whose nodes are "+ + "exhausted partway through the left traversal "+ + "(leafIndex stuck before proof.Start())") +} + +// TestValidateCompleteness_RightSideCheck_StillRejectsSmallNamespace is a +// control test: it confirms the right-side namespace check itself still +// works and that the fix only rejects the truncated-proof case, not the +// well-formed case where start==0 and a right subtree's namespace is +// <= targetNID. +func TestValidateCompleteness_RightSideCheck_StillRejectsSmallNamespace(t *testing.T) { + nth := NewNmtHasher(sha256.New(), namespace.IDSize(1), false) + targetNID := namespace.ID{0x05} + + // Right subtree with min-namespace 0x01, which is <= targetNID 0x05, + // so the right-side check must reject it. + smallNID := []byte{0x01} + digest := sha256.Sum256([]byte("rightSubtreeNode")) + rightNode := append(append([]byte{}, smallNID...), smallNID...) + rightNode = append(rightNode, digest[:]...) + + // start=0 means the left-traversal loop exits immediately with + // leafIndex == proof.Start() == 0, so the new insufficiency check + // does not trip. The sole node becomes a rightSubtree. + proof := Proof{ + start: 0, + end: 1, + nodes: [][]byte{rightNode}, + } + + err := proof.validateCompleteness(nth, targetNID) + require.ErrorIs(t, err, ErrFailedCompletenessCheck) +} From fd647f5c9a55639edfd60564e8f753e4fb28c16e Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Mon, 13 Apr 2026 07:35:45 -0700 Subject: [PATCH 2/2] fix: wrap ErrFailedCompletenessCheck for the truncated-proof branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on #317. The truncated-proof failure is semantically a completeness-check failure — same category as the two existing returns in this function — so wrap the sentinel with %w and assert identity via errors.Is/require.ErrorIs. Co-Authored-By: Claude Opus 4.6 (1M context) --- proof.go | 4 ++-- proof_completeness_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/proof.go b/proof.go index b5ebe03..a02a2b6 100644 --- a/proof.go +++ b/proof.go @@ -322,8 +322,8 @@ func (proof Proof) validateCompleteness(nth *NmtHasher, nID namespace.ID) error // silently skipped — see GHSA-r9fq-g486-v8pg. if leafIndex != uint64(proof.Start()) { return fmt.Errorf( - "proof nodes insufficient: left traversal reached leaf index %d, expected %d", - leafIndex, proof.Start(), + "%w: proof nodes insufficient: left traversal reached leaf index %d, expected %d", + ErrFailedCompletenessCheck, leafIndex, proof.Start(), ) } // rightSubtrees only contains the subtrees after r.End diff --git a/proof_completeness_test.go b/proof_completeness_test.go index b708d11..57ba858 100644 --- a/proof_completeness_test.go +++ b/proof_completeness_test.go @@ -27,7 +27,7 @@ func TestValidateCompleteness_TruncatedProof_RejectsEmptyNodes(t *testing.T) { } err := proof.validateCompleteness(nth, targetNID) - require.Error(t, err, + require.ErrorIs(t, err, ErrFailedCompletenessCheck, "validateCompleteness must reject a proof whose nodes are "+ "exhausted before the left traversal reaches proof.Start()") } @@ -60,7 +60,7 @@ func TestValidateCompleteness_TruncatedProof_RejectsInsufficientNodes(t *testing } err := proof.validateCompleteness(nth, targetNID) - require.Error(t, err, + require.ErrorIs(t, err, ErrFailedCompletenessCheck, "validateCompleteness must reject a proof whose nodes are "+ "exhausted partway through the left traversal "+ "(leafIndex stuck before proof.Start())")