Merge pull request #1300 from tendermint/lite-proxy-hardening-and-tests

lite/proxy: Validation* tests and hardening for nil dereferences

lite/proxy/block.go

@@ -11,11 +11,17 @@ import (
 )
 
 func ValidateBlockMeta(meta *types.BlockMeta, check lite.Commit) error {
+	if meta == nil {
+		return errors.New("expecting a non-nil BlockMeta")
+	}
 	// TODO: check the BlockID??
 	return ValidateHeader(meta.Header, check)
 }
 
 func ValidateBlock(meta *types.Block, check lite.Commit) error {
+	if meta == nil {
+		return errors.New("expecting a non-nil Block")
+	}
 	err := ValidateHeader(meta.Header, check)
 	if err != nil {
 		return err
@@ -27,6 +33,9 @@ func ValidateBlock(meta *types.Block, check lite.Commit) error {
 }
 
 func ValidateHeader(head *types.Header, check lite.Commit) error {
+	if head == nil {
+		return errors.New("expecting a non-nil Header")
+	}
 	// make sure they are for the same height (obvious fail)
 	if head.Height != check.Height() {
 		return certerr.ErrHeightMismatch(head.Height, check.Height())

lite/proxy/validate_test.go

@@ -0,0 +1,218 @@
+package proxy_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/tendermint/tendermint/lite"
+	"github.com/tendermint/tendermint/lite/proxy"
+	"github.com/tendermint/tendermint/types"
+)
+
+var (
+	deadBeefTxs  = types.Txs{[]byte("DE"), []byte("AD"), []byte("BE"), []byte("EF")}
+	deadBeefHash = deadBeefTxs.Hash()
+	testTime1    = time.Date(2018, 1, 1, 1, 1, 1, 1, time.UTC)
+	testTime2    = time.Date(2017, 1, 2, 1, 1, 1, 1, time.UTC)
+)
+
+var hdrHeight11 = &types.Header{
+	Height:         11,
+	Time:           testTime1,
+	ValidatorsHash: []byte("Tendermint"),
+}
+
+func TestValidateBlock(t *testing.T) {
+	tests := []struct {
+		block   *types.Block
+		commit  lite.Commit
+		wantErr string
+	}{
+		{
+			block: nil, wantErr: "non-nil Block",
+		},
+		{
+			block: &types.Block{}, wantErr: "nil Header",
+		},
+		{
+			block: &types.Block{Header: new(types.Header)},
+		},
+
+		// Start Header.Height mismatch test
+		{
+			block:   &types.Block{Header: &types.Header{Height: 10}},
+			commit:  lite.Commit{Header: &types.Header{Height: 11}},
+			wantErr: "don't match - 10 vs 11",
+		},
+
+		{
+			block:  &types.Block{Header: &types.Header{Height: 11}},
+			commit: lite.Commit{Header: &types.Header{Height: 11}},
+		},
+		// End Header.Height mismatch test
+
+		// Start Header.Hash mismatch test
+		{
+			block:   &types.Block{Header: hdrHeight11},
+			commit:  lite.Commit{Header: &types.Header{Height: 11}},
+			wantErr: "Headers don't match",
+		},
+
+		{
+			block:  &types.Block{Header: hdrHeight11},
+			commit: lite.Commit{Header: hdrHeight11},
+		},
+		// End Header.Hash mismatch test
+
+		// Start Header.Data hash mismatch test
+		{
+			block: &types.Block{
+				Header: &types.Header{Height: 11},
+				Data:   &types.Data{Txs: []types.Tx{[]byte("0xDE"), []byte("AD")}},
+			},
+			commit: lite.Commit{
+				Header: &types.Header{Height: 11},
+				Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("0xDEADBEEF")}},
+			},
+			wantErr: "Data hash doesn't match header",
+		},
+		{
+			block: &types.Block{
+				Header: &types.Header{Height: 11, DataHash: deadBeefHash},
+				Data:   &types.Data{Txs: deadBeefTxs},
+			},
+			commit: lite.Commit{
+				Header: &types.Header{Height: 11},
+				Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}},
+			},
+		},
+		// End Header.Data hash mismatch test
+	}
+
+	for i, tt := range tests {
+		err := proxy.ValidateBlock(tt.block, tt.commit)
+		if tt.wantErr != "" {
+			if err == nil {
+				assert.FailNowf(t, "Unexpectedly passed", "#%d", i)
+			} else {
+				assert.Contains(t, err.Error(), tt.wantErr, "#%d should contain the substring\n\n", i)
+			}
+			continue
+		}
+
+		assert.Nil(t, err, "#%d: expecting a nil error", i)
+	}
+}
+
+func TestValidateBlockMeta(t *testing.T) {
+	tests := []struct {
+		meta    *types.BlockMeta
+		commit  lite.Commit
+		wantErr string
+	}{
+		{
+			meta: nil, wantErr: "non-nil BlockMeta",
+		},
+		{
+			meta: &types.BlockMeta{}, wantErr: "non-nil Header",
+		},
+		{
+			meta: &types.BlockMeta{Header: new(types.Header)},
+		},
+
+		// Start Header.Height mismatch test
+		{
+			meta:    &types.BlockMeta{Header: &types.Header{Height: 10}},
+			commit:  lite.Commit{Header: &types.Header{Height: 11}},
+			wantErr: "don't match - 10 vs 11",
+		},
+
+		{
+			meta:   &types.BlockMeta{Header: &types.Header{Height: 11}},
+			commit: lite.Commit{Header: &types.Header{Height: 11}},
+		},
+		// End Header.Height mismatch test
+
+		// Start Headers don't match test
+		{
+			meta:    &types.BlockMeta{Header: hdrHeight11},
+			commit:  lite.Commit{Header: &types.Header{Height: 11}},
+			wantErr: "Headers don't match",
+		},
+
+		{
+			meta:   &types.BlockMeta{Header: hdrHeight11},
+			commit: lite.Commit{Header: hdrHeight11},
+		},
+
+		{
+			meta: &types.BlockMeta{
+				Header: &types.Header{
+					Height:         11,
+					ValidatorsHash: []byte("lite-test"),
+					// TODO: should be able to use empty time after Amino upgrade
+					Time: testTime1,
+				},
+			},
+			commit: lite.Commit{
+				Header: &types.Header{Height: 11, DataHash: deadBeefHash},
+			},
+			wantErr: "Headers don't match",
+		},
+
+		{
+			meta: &types.BlockMeta{
+				Header: &types.Header{
+					Height: 11, DataHash: deadBeefHash,
+					ValidatorsHash: []byte("Tendermint"),
+					Time:           testTime1,
+				},
+			},
+			commit: lite.Commit{
+				Header: &types.Header{
+					Height: 11, DataHash: deadBeefHash,
+					ValidatorsHash: []byte("Tendermint"),
+					Time:           testTime2,
+				},
+				Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}},
+			},
+			wantErr: "Headers don't match",
+		},
+
+		{
+			meta: &types.BlockMeta{
+				Header: &types.Header{
+					Height: 11, DataHash: deadBeefHash,
+					ValidatorsHash: []byte("Tendermint"),
+					Time:           testTime2,
+				},
+			},
+			commit: lite.Commit{
+				Header: &types.Header{
+					Height: 11, DataHash: deadBeefHash,
+					ValidatorsHash: []byte("Tendermint-x"),
+					Time:           testTime2,
+				},
+				Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}},
+			},
+			wantErr: "Headers don't match",
+		},
+		// End Headers don't match test
+	}
+
+	for i, tt := range tests {
+		err := proxy.ValidateBlockMeta(tt.meta, tt.commit)
+		if tt.wantErr != "" {
+			if err == nil {
+				assert.FailNowf(t, "Unexpectedly passed", "#%d: wanted error %q", i, tt.wantErr)
+			} else {
+				assert.Contains(t, err.Error(), tt.wantErr, "#%d should contain the substring\n\n", i)
+			}
+			continue
+		}
+
+		assert.Nil(t, err, "#%d: expecting a nil error", i)
+	}
+}

types/block.go

@@ -176,7 +176,9 @@ type Header struct {
 }
 
 // Hash returns the hash of the header.
-// Returns nil if ValidatorHash is missing.
+// Returns nil if ValidatorHash is missing,
+// since a Header is not valid unless there is
+// a ValidaotrsHash (corresponding to the validator set).
 func (h *Header) Hash() cmn.HexBytes {
 	if h == nil || len(h.ValidatorsHash) == 0 {
 		return nil