light: implement validate basic (#4916)

run a validate basic on inbound validator sets and headers before further processing them

CHANGELOG_PENDING.md

@@ -48,7 +48,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 
 - [statesync] Add state sync support, where a new node can be rapidly bootstrapped by fetching state snapshots from peers instead of replaying blocks. See the `[statesync]` config section.
 - [evidence] [\#4532](https://github.com/tendermint/tendermint/pull/4532) Handle evidence from light clients (@melekes)
-- [lite2] [\#4532](https://github.com/tendermint/tendermint/pull/4532) Submit conflicting headers, if any, to a full node & all witnesses (@melekes)
+- [light] [\#4532](https://github.com/tendermint/tendermint/pull/4532) Submit conflicting headers, if any, to a full node & all witnesses (@melekes)
 - [rpc] [\#4532](https://github.com/tendermint/tendermint/pull/4923) Support `BlockByHash` query (@fedekunze)
 
 ### IMPROVEMENTS:
@@ -63,8 +63,9 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 - [types] [\#4905](https://github.com/tendermint/tendermint/pull/4905) Add ValidateBasic to validator and validator set (@cmwaters)
 - [consensus] [\#4578](https://github.com/tendermint/tendermint/issues/4578) Attempt to repair the consensus WAL file (`data/cs.wal/wal`) automatically in case of corruption (@alessio)
   The original WAL file will be backed up to `data/cs.wal/wal.CORRUPTED`.
-- [lite2] [\#4935](https://github.com/tendermint/tendermint/pull/4935) Fetch and compare a new header with witnesses in parallel (@melekes)
-- [lite2] [\#4929](https://github.com/tendermint/tendermint/pull/4929) compare header w/ witnesses only when doing bisection (@melekes)
+- [light] [\#4935](https://github.com/tendermint/tendermint/pull/4935) Fetch and compare a new header with witnesses in parallel (@melekes)
+- [light] [\#4929](https://github.com/tendermint/tendermint/pull/4929) compare header w/ witnesses only when doing bisection (@melekes)
+- [light] [\#4916](https://github.com/tendermint/tendermint/pull/4916) validate basic for inbound validator sets and headers before further processing them (@cmwaters)
 
 ### BUG FIXES:
 

light/client.go

@@ -1071,6 +1071,7 @@ func (c *Client) replacePrimaryProvider() error {
 	c.providerMutex.Lock()
 	defer c.providerMutex.Unlock()
 
+	c.logger.Info("Primary is unavailable. Replacing with the first witness")
 	if len(c.witnesses) <= 1 {
 		return errNoWitnesses{}
 	}
@@ -1087,55 +1088,89 @@ func (c *Client) replacePrimaryProvider() error {
 func (c *Client) signedHeaderFromPrimary(height int64) (*types.SignedHeader, error) {
 	for attempt := uint16(1); attempt <= c.maxRetryAttempts; attempt++ {
 		c.providerMutex.Lock()
-		h, err := c.primary.SignedHeader(height)
+		h, providerErr := c.primary.SignedHeader(height)
 		c.providerMutex.Unlock()
-		if err == nil {
-			// sanity check
-			if height > 0 && h.Height != height {
-				return nil, fmt.Errorf("expected %d height, got %d", height, h.Height)
+		if providerErr == nil {
+			err := c.validateHeader(h, height)
+			if err != nil {
+				replaceErr := c.replacePrimaryProvider()
+				if replaceErr != nil {
+					return nil, fmt.Errorf("%v. Tried to replace primary but: %w", err.Error(), replaceErr)
+				}
+				// replace primary and request signed header again
+				return c.signedHeaderFromPrimary(height)
 			}
+			// valid header has been received
 			return h, nil
 		}
-		if err == provider.ErrSignedHeaderNotFound {
-			return nil, err
+		if providerErr == provider.ErrSignedHeaderNotFound {
+			return nil, providerErr
 		}
-		c.logger.Error("Failed to get signed header from primary", "attempt", attempt, "err", err)
+		c.logger.Error("Failed to get signed header from primary", "attempt", attempt, "err", providerErr)
 		time.Sleep(backoffTimeout(attempt))
 	}
 
-	c.logger.Info("Primary is unavailable. Replacing with the first witness")
 	err := c.replacePrimaryProvider()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("primary dropped out. Tried to replace but: %w", err)
 	}
 
 	return c.signedHeaderFromPrimary(height)
 }
 
+func (c *Client) validateHeader(h *types.SignedHeader, expectedHeight int64) error {
+	if h == nil {
+		return errors.New("nil header")
+	}
+	if expectedHeight > 0 && h.Height != expectedHeight {
+		return errors.New("height mismatch")
+	}
+	return h.ValidateBasic(c.chainID)
+}
+
 // validatorSetFromPrimary retrieves the ValidatorSet from the primary provider
 // at the specified height. Handles dropout by the primary provider after 5
 // attempts by replacing it with an alternative provider.
 func (c *Client) validatorSetFromPrimary(height int64) (*types.ValidatorSet, error) {
 	for attempt := uint16(1); attempt <= c.maxRetryAttempts; attempt++ {
 		c.providerMutex.Lock()
-		vals, err := c.primary.ValidatorSet(height)
+		vals, providerErr := c.primary.ValidatorSet(height)
 		c.providerMutex.Unlock()
-		if err == nil || err == provider.ErrValidatorSetNotFound {
-			return vals, err
+		if providerErr == nil {
+			err := c.validateValidatorSet(vals)
+			if err != nil {
+				replaceErr := c.replacePrimaryProvider()
+				if replaceErr != nil {
+					return nil, fmt.Errorf("%v. Tried to replace primary but: %w", err.Error(), replaceErr)
+				}
+				// replace primary and request signed header again
+				return c.validatorSetFromPrimary(height)
+			}
+			return vals, nil
 		}
-		c.logger.Error("Failed to get validator set from primary", "attempt", attempt, "err", err)
+
+		if providerErr == provider.ErrValidatorSetNotFound {
+			return vals, providerErr
+		}
+		c.logger.Error("Failed to get validator set from primary", "attempt", attempt, "err", providerErr)
 		time.Sleep(backoffTimeout(attempt))
 	}
 
-	c.logger.Info("Primary is unavailable. Replacing with the first witness")
 	err := c.replacePrimaryProvider()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("primary dropped out. Tried to replace but: %w", err)
 	}
 
 	return c.validatorSetFromPrimary(height)
 }
 
+func (c *Client) validateValidatorSet(vals *types.ValidatorSet) error {
+	if vals == nil {
+		return errors.New("validator set is nil")
+	}
+	return vals.ValidateBasic()
+}
+
 // sendConflictingHeadersEvidence sends evidence to all witnesses and primary
 // on best effort basis.
 //

light/client_test.go

@@ -973,3 +973,74 @@ func TestClientReportsConflictingHeadersEvidence(t *testing.T) {
 	assert.True(t, fullNode2.HasEvidence(ev))
 	assert.True(t, fullNode.HasEvidence(ev))
 }
+
+func TestClientEnsureValidHeadersAndValSets(t *testing.T) {
+	emptyValSet := &types.ValidatorSet{
+		Validators: nil,
+		Proposer:   nil,
+	}
+
+	testCases := []struct {
+		headers map[int64]*types.SignedHeader
+		vals    map[int64]*types.ValidatorSet
+		err     bool
+	}{
+		{
+			headerSet,
+			valSet,
+			false,
+		},
+		{
+			headerSet,
+			map[int64]*types.ValidatorSet{
+				1: vals,
+				2: vals,
+				3: nil,
+			},
+			true,
+		},
+		{
+			map[int64]*types.SignedHeader{
+				1: h1,
+				2: h2,
+				3: nil,
+			},
+			valSet,
+			true,
+		},
+		{
+			headerSet,
+			map[int64]*types.ValidatorSet{
+				1: vals,
+				2: vals,
+				3: emptyValSet,
+			},
+			true,
+		},
+	}
+
+	for _, tc := range testCases {
+		badNode := mockp.New(
+			chainID,
+			tc.headers,
+			tc.vals,
+		)
+		c, err := light.NewClient(
+			chainID,
+			trustOptions,
+			badNode,
+			[]provider.Provider{badNode, badNode},
+			dbs.New(dbm.NewMemDB(), chainID),
+			light.MaxRetryAttempts(1),
+		)
+		require.NoError(t, err)
+
+		_, err = c.VerifyHeaderAtHeight(3, bTime.Add(2*time.Hour))
+		if tc.err {
+			assert.Error(t, err)
+		} else {
+			assert.NoError(t, err)
+		}
+	}
+
+}