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.
  • Loading branch information
odeke-em authored Mar 6, 2024
1 parent 9a920ba commit adb7a57
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
21 changes: 21 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/test/util/blobfactory"
Expand Down Expand Up @@ -256,3 +259,21 @@ func TestAllSharesInclusionProof(t *testing.T) {
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")
}
}
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 adb7a57

Please sign in to comment.