Skip to content

Commit

Permalink
fix(blob): add fixes along with the fuzzers that discovered them
Browse files Browse the repository at this point in the history
This changes adds fuzzers+corpra that found some bugs, along
with tests and reproducers to catch future regressions.

Fixes #3727
Fixes #3728
Fixes #3729
Fixes #3730
Fixes #3731
  • Loading branch information
odeke-em committed Sep 16, 2024
1 parent 1a1286f commit 9305568
Show file tree
Hide file tree
Showing 18 changed files with 198 additions and 0 deletions.
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
86 changes: 86 additions & 0 deletions blob/blob_fuzz_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package blob

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

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

func FuzzProofEqual(f *testing.F) {
if testing.Short() {
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)
})
}

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) {
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) {
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.

2 changes: 2 additions & 0 deletions blob/testdata/fuzz/FuzzCommitmentProofVerify/582528ddfad69eb5
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("0")
2 changes: 2 additions & 0 deletions blob/testdata/fuzz/FuzzCommitmentProofVerify/6a2b4b982dc67bf9
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}")
2 changes: 2 additions & 0 deletions blob/testdata/fuzz/FuzzCommitmentProofVerify/8093511184ad3e25
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
go test fuzz v1
[]byte("{}")
2 changes: 2 additions & 0 deletions blob/testdata/fuzz/FuzzCommitmentProofVerify/c23296029c66526c
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}")
2 changes: 2 additions & 0 deletions blob/testdata/fuzz/FuzzCommitmentProofVerify/ea1afcc1fa86d415
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}")
2 changes: 2 additions & 0 deletions blob/testdata/fuzz/FuzzCommitmentProofVerify/fa5be9420bbacd9a
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("{}")

0 comments on commit 9305568

Please sign in to comment.