blob: CommitmentProof.Verify should check and ensure that subtreeRootThreshold is >= 1 lest a divide-by-zero-panic

[odeke-em]

Celestia Node version

1a1286f

OS

darwin

Install tools

No response

Others

No response

Steps to reproduce it

While fuzzing the code for CommitmentProof.Verify, if I run this code

package blob_test       

import (
        "testing"
        
        "github.com/celestiaorg/celestia-node/blob"
)       
                        
func TestCommitmentProofVerifyZeroSubThreshold(t *testing.T) {
        cp := new(blob.CommitmentProof)
        _, _ = cp.Verify(nil, 0)
}

we get a panic

Expected result

No panic and instead an error

Actual result

panic: runtime error: integer divide by zero
            goroutine 9589 [running]:
            runtime/debug.Stack()
            	/Users/emmanuelodeke/go/pkg/mod/golang.org/[email protected]/src/runtime/debug/stack.go:26 +0x9b
            testing.tRunner.func1()
            	/Users/emmanuelodeke/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1591 +0x1c8
            panic({0x106c66820?, 0x108121230?})
            	/Users/emmanuelodeke/go/pkg/mod/golang.org/[email protected]/src/runtime/panic.go:785 +0x132
            github.com/celestiaorg/go-square/inclusion.SubTreeWidth(0xc0002596b7?, 0x1?)
            	/Users/emmanuelodeke/go/pkg/mod/github.com/celestiaorg/[email protected]/inclusion/blob_share_commitment_rules.go:63 +0x212
            github.com/celestiaorg/celestia-node/blob.(*CommitmentProof).Verify(0xc00c545c20, {0x0, 0x0, 0x0}, 0x0)
            	/Users/emmanuelodeke/go/src/github.com/celestiaorg/celestia-node/blob/commitment_proof.go:101 +0x25a
            github.com/celestiaorg/celestia-node/blob.FuzzCommitmentProofVerify.func1(0x0?, {0xc00112fb00, 0x17, 0xd80})
            	/Users/emmanuelodeke/go/src/github.com/celestiaorg/celestia-node/blob/blob_fuzz_test.go:83 +0xfb
            reflect.Value.call({0x106bca9a0?, 0x106f64180?, 0x13?}, {0x1060d7eca, 0x4}, {0xc00d0580f0, 0x2, 0x2?})
            	/Users/emmanuelodeke/go/pkg/mod/golang.org/[email protected]/src/reflect/value.go:581 +0xca6
            reflect.Value.Call({0x106bca9a0?, 0x106f64180?, 0x103a35ded?}, {0xc00d0580f0?, 0x106f5fe00?, 0xf?})
            	/Users/emmanuelodeke/go/pkg/mod/golang.org/[email protected]/src/reflect/value.go:365 +0xb9
            testing.(*F).Fuzz.func1.1(0xc00d053ba0?)
            	/Users/emmanuelodeke/go/pkg/mod/golang.org/[email protected]/src/testing/fuzz.go:335 +0x305
            testing.tRunner(0xc00d053ba0, 0xc00d055320)
            	/Users/emmanuelodeke/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1690 +0xf4
            created by testing.(*F).Fuzz.func1 in goroutine 22
            	/Users/emmanuelodeke/go/pkg/mod/golang.org/[email protected]/src/testing/fuzz.go:322 +0x577

Suggested fix

diff --git a/blob/commitment_proof.go b/blob/commitment_proof.go
index 8fa74671..f9d1af06 100644
--- a/blob/commitment_proof.go
+++ b/blob/commitment_proof.go
@@ -2,6 +2,7 @@ package blob
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 
 	"github.com/celestiaorg/celestia-app/v2/pkg/appconsts"
@@ -93,6 +94,10 @@ func (commitmentProof *CommitmentProof) Verify(root []byte, subtreeRootThreshold
 		numberOfShares += proof.End() - proof.Start()
 	}
 
+	if subtreeRootThreshold <= 0 {
+		return false, errors.New("subtreeRootThreshould must be > 0")
+	}
+
 	// use the computed total number of shares to calculate the subtree roots
 	// width.
 	// the subtree roots width is defined in ADR-013:

/cc @liamsi @rootulp @musalbas

[odeke-em]

I have mailed out a PR with the fixes along with the fuzzers per #3732