Skip to content

Commit

Permalink
fix(pkg/proof): reject negative indices in QueryTxInclusionProof (#3141)
Browse files Browse the repository at this point in the history
This change ensures that any negative indices are outright rejected to
avoid any future overflows due to the wrapping around of uint64(v) and
control from and outside caller.

Fixes #3140.

(cherry picked from commit adb7a57)

# Conflicts:
#	pkg/proof/proof_test.go
  • Loading branch information
odeke-em authored and mergify[bot] committed Mar 6, 2024
1 parent b25766f commit 0bf6dfb
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 0 deletions.
56 changes: 56 additions & 0 deletions pkg/proof/proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package proof_test

import (
"bytes"
"strings"
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
abci "github.com/tendermint/tendermint/abci/types"
tmrand "github.com/tendermint/tendermint/libs/rand"

"github.com/celestiaorg/celestia-app/app"
Expand Down Expand Up @@ -221,3 +224,56 @@ func TestNewShareInclusionProof(t *testing.T) {
})
}
}
<<<<<<< HEAD

Check failure on line 227 in pkg/proof/proof_test.go

View workflow job for this annotation

GitHub Actions / lint / golangci-lint

syntax error: non-declaration statement outside function body

Check failure on line 227 in pkg/proof/proof_test.go

View workflow job for this annotation

GitHub Actions / lint / golangci-lint

expected declaration, found '<<' (typecheck)

Check failure on line 227 in pkg/proof/proof_test.go

View workflow job for this annotation

GitHub Actions / test / test-short

expected declaration, found '<<'

Check failure on line 227 in pkg/proof/proof_test.go

View workflow job for this annotation

GitHub Actions / test / test

expected declaration, found '<<'

Check failure on line 227 in pkg/proof/proof_test.go

View workflow job for this annotation

GitHub Actions / test / test-coverage

expected declaration, found '<<'

Check failure on line 227 in pkg/proof/proof_test.go

View workflow job for this annotation

GitHub Actions / test / test-race

expected declaration, found '<<'
=======

// TestAllSharesInclusionProof creates a proof for all shares in the data
// square. Since we can't prove multiple namespaces at the moment, all the
// shares use the same namespace.
func TestAllSharesInclusionProof(t *testing.T) {
txs := testfactory.GenerateRandomTxs(243, 500)

dataSquare, err := square.Construct(txs.ToSliceOfBytes(), appconsts.SquareSizeUpperBound(appconsts.LatestVersion), appconsts.SubtreeRootThreshold(appconsts.LatestVersion))
require.NoError(t, err)
assert.Equal(t, 256, len(dataSquare))

// erasure the data square which we use to create the data root.
eds, err := da.ExtendShares(shares.ToBytes(dataSquare))
require.NoError(t, err)

// create the new data root by creating the data availability header (merkle
// roots of each row and col of the erasure data).
dah, err := da.NewDataAvailabilityHeader(eds)
require.NoError(t, err)
dataRoot := dah.Hash()

actualNamespace, err := proof.ParseNamespace(dataSquare, 0, 256)
require.NoError(t, err)
require.Equal(t, appns.TxNamespace, actualNamespace)
proof, err := proof.NewShareInclusionProof(
dataSquare,
appns.TxNamespace,
shares.NewRange(0, 256),
)
require.NoError(t, err)
assert.NoError(t, proof.Validate(dataRoot))
}

// Ensure that we reject negative index values and avoid overflows.
// https://github.com/celestiaorg/celestia-app/issues/3140
func TestQueryTxInclusionProofRejectsNegativeValues(t *testing.T) {
path := []string{"-2"}
req := abci.RequestQuery{Data: []byte{}}
ctx := sdk.Context{}
rawProof, err := proof.QueryTxInclusionProof(ctx, path, req)
if err == nil {
t.Fatal("expected a non-nil error")
}
if !strings.Contains(err.Error(), "negative") {
t.Fatalf("The error should reject negative values and report such, but did not\n\tGot: %v", err)
}
if len(rawProof) != 0 {
t.Fatal("no rawProof expected")
}
}
>>>>>>> adb7a570 (fix(pkg/proof): reject negative indices in QueryTxInclusionProof (#3141))

Check failure on line 279 in pkg/proof/proof_test.go

View workflow job for this annotation

GitHub Actions / lint / golangci-lint

syntax error: non-declaration statement outside function body

Check failure on line 279 in pkg/proof/proof_test.go

View workflow job for this annotation

GitHub Actions / lint / golangci-lint

invalid character U+0023 '#' (typecheck)

Check failure on line 279 in pkg/proof/proof_test.go

View workflow job for this annotation

GitHub Actions / lint / golangci-lint

illegal character U+0023 '#' (typecheck)
3 changes: 3 additions & 0 deletions pkg/proof/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ func QueryTxInclusionProof(_ sdk.Context, path []string, req abci.RequestQuery)
if err != nil {
return nil, err
}
if index < 0 {
return nil, fmt.Errorf("path[0] element: %q produced a negative value: %d", path[0], index)
}

// unmarshal the block data that is passed from the ABCI client
pbb := new(tmproto.Block)
Expand Down

0 comments on commit 0bf6dfb

Please sign in to comment.