Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(blob): add fixes along with the fuzzers that discovered them #3732

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions blob/blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ func (b *Blob) UnmarshalJSON(data []byte) error {
return err
}

if len(jsonBlob.Namespace) == 0 {
return errors.New("expected a non-empty namespace")
}

b.Blob = &blob.Blob{}
b.Blob.NamespaceVersion = uint32(jsonBlob.Namespace.Version())
b.Blob.NamespaceId = jsonBlob.Namespace.ID()
Expand Down
92 changes: 92 additions & 0 deletions blob/blob_fuzz_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package blob

import (
"encoding/json"
"os"
"path/filepath"
"testing"

"github.com/celestiaorg/celestia-node/blob/blobtest"
)

func FuzzProofEqual(f *testing.F) {
if testing.Short() {
cristaloleg marked this conversation as resolved.
Show resolved Hide resolved
f.Skip("in -short mode")
}

// 1. Generate the corpus.
squareBlobs, err := blobtest.GenerateV0Blobs([]int{16}, false)
if err != nil {
f.Fatal(err)
}

blobs, err := convertBlobs(squareBlobs...)
if err != nil {
f.Fatal(err)
}

for _, blob := range blobs {
jsonBlob, err := blob.MarshalJSON()
if err != nil {
f.Fatal(err)
}
f.Add(jsonBlob)
}

// 2. Run the fuzzer.
f.Fuzz(func(t *testing.T, jsonBlob []byte) {
blob := new(Blob)
_ = blob.UnmarshalJSON(jsonBlob)
})
}

type verifyCorpus struct {
CP *CommitmentProof `json:"commitment_proof"`
Root []byte `json:"root"`
SThreshold int `json:"sub_threshold"`
}

func FuzzCommitmentProofVerify(f *testing.F) {
if testing.Short() {
f.Skip("in -short mode")
}

path := filepath.Join("testdata", "fuzz-corpus", "verify-*.json")
paths, err := filepath.Glob(path)
if err != nil {
f.Fatal(err)
}

// Add the corpra.
for _, path := range paths {
blob, err := os.ReadFile(path)
if err != nil {
f.Fatal(err)
}

var values []*verifyCorpus
if err := json.Unmarshal(blob, &values); err != nil {
f.Fatal(err)
}

for _, value := range values {
blob, err := json.Marshal(value)
if err != nil {
f.Fatal(err)
}
f.Add(blob)
}
}

f.Fuzz(func(t *testing.T, valueJSON []byte) {
val := new(verifyCorpus)
if err := json.Unmarshal(valueJSON, val); err != nil {
return
}
commitProof := val.CP
if commitProof == nil {
return
}
_, _ = commitProof.Verify(val.Root, val.SThreshold)
})
}
23 changes: 23 additions & 0 deletions blob/commitment_proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package blob

import (
"bytes"
"errors"
"fmt"

"github.com/celestiaorg/celestia-app/v2/pkg/appconsts"
Expand Down Expand Up @@ -85,6 +86,15 @@ func (commitmentProof *CommitmentProof) Validate() error {
// Expects the commitment proof to be properly formulated and validated
// using the Validate() function.
func (commitmentProof *CommitmentProof) Verify(root []byte, subtreeRootThreshold int) (bool, error) {
if len(root) == 0 {
return false, errors.New("root must be non-empty")
}

rp := commitmentProof.RowProof
if err := rp.Validate(root); err != nil {
return false, err
}

nmtHasher := nmt.NewNmtHasher(appconsts.NewBaseHashFunc(), share.NamespaceSize, true)

// computes the total number of shares proven.
Expand All @@ -93,6 +103,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:
Expand All @@ -108,6 +122,15 @@ func (commitmentProof *CommitmentProof) Verify(root []byte, subtreeRootThreshold
if err != nil {
return false, err
}

if len(commitmentProof.SubtreeRoots) < subtreeRootsCursor {
return false, fmt.Errorf("len(commitmentProof.SubtreeRoots)=%d < subtreeRootsCursor=%d",
len(commitmentProof.SubtreeRoots), subtreeRootsCursor)
}
if len(commitmentProof.SubtreeRoots) < subtreeRootsCursor+len(ranges) {
cristaloleg marked this conversation as resolved.
Show resolved Hide resolved
return false, fmt.Errorf("len(commitmentProof.SubtreeRoots)=%d < subtreeRootsCursor+len(ranges)=%d",
len(commitmentProof.SubtreeRoots), subtreeRootsCursor+len(ranges))
}
valid, err := subtreeRootProof.VerifySubtreeRootInclusion(
nmtHasher,
commitmentProof.SubtreeRoots[subtreeRootsCursor:subtreeRootsCursor+len(ranges)],
Expand Down
64 changes: 64 additions & 0 deletions blob/repro_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package blob_test

import (
"testing"

"github.com/celestiaorg/celestia-app/v2/pkg/proof"
"github.com/celestiaorg/celestia-node/blob"
"github.com/celestiaorg/nmt"
"github.com/celestiaorg/nmt/pb"
)

// Reported at https://github.com/celestiaorg/celestia-node/issues/3731.
func TestCommitmentProofRowProofVerifyWithEmptyRoot(t *testing.T) {
cristaloleg marked this conversation as resolved.
Show resolved Hide resolved
cp := &blob.CommitmentProof{
RowProof: proof.RowProof{
Proofs: []*proof.Proof{{}},
},
}
root := []byte{0xd3, 0x4d, 0x34}
if _, err := cp.Verify(root, 1); err == nil {
t.Fatal("expected a non-nil error")
}
}

// Reported at https://github.com/celestiaorg/celestia-node/issues/3730.
func TestCommitmentProofRowProofVerify(t *testing.T) {
cp := &blob.CommitmentProof{
RowProof: proof.RowProof{
Proofs: []*proof.Proof{{}},
},
}
if _, err := cp.Verify(nil, 1); err == nil {
t.Fatal("expected a non-nil error")
}
}

// Reported at https://github.com/celestiaorg/celestia-node/issues/3729.
func TestCommitmentProofVerifySliceBound(t *testing.T) {
proof := nmt.ProtoToProof(pb.Proof{End: 1})
cp := &blob.CommitmentProof{
SubtreeRootProofs: []*nmt.Proof{
&proof,
},
}
if _, err := cp.Verify(nil, 1); err == nil {
t.Fatal("expected a non-nil error")
}
}

// Reported at https://github.com/celestiaorg/celestia-node/issues/3728.
func TestCommitmentProofVerifyZeroSubThreshold(t *testing.T) {
cp := new(blob.CommitmentProof)
if _, err := cp.Verify(nil, 0); err == nil {
t.Fatal("expected a non-nil error")
}
}

// Reported at https://github.com/celestiaorg/celestia-node/issues/3727.
func TestBlobUnmarshalRepro(t *testing.T) {
blob := new(blob.Blob)
if err := blob.UnmarshalJSON([]byte("{}")); err == nil {
t.Fatal("expected a non-nil error")
}
}

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("0")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("{\"Commitment_proof\":{\"suBtree_root_proofs\":[{\"end\":1}]},\"suB_threshold\":1}")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("{}")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("{\"Commitment_proof\":{\"row_proof\":{\"proofs\":[{}]}},\"root\":\"0000\",\"suB_threshold\":1}")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("{\"Commitment_proof\":{\"row_proof\":{\"proofs\":[{}]}},\"suB_threshold\":1}")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("{\"Commitment_proof\":{}}")
2 changes: 2 additions & 0 deletions blob/testdata/fuzz/FuzzProofEqual/8093511184ad3e25
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("{}")
Loading