From 4c2f56626ac3161d871bd4b44ec80ea0ff3b1284 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Sat, 10 Mar 2018 21:15:55 -0800 Subject: [PATCH] lite/proxy: Validation* tests and hardening for nil dereferences Updates https://github.com/tendermint/tendermint/issues/1017 Ensure that the Validate* functions in proxy are tests and cover the case of sneakish bugs that have been encountered a few times from nil dereferences. The lite package should theoretically never panic with a nil dereference. It is meant to contain the certifiers hence it should never panic with such. Requires the following bugs to be fixed first; * https://github.com/tendermint/tendermint/issues/1298 * https://github.com/tendermint/tendermint/issues/1299 --- lite/proxy/block.go | 9 ++ lite/proxy/validate_test.go | 256 ++++++++++++++++++++++++++++++++++++ 2 files changed, 265 insertions(+) create mode 100644 lite/proxy/validate_test.go diff --git a/lite/proxy/block.go b/lite/proxy/block.go index 60cd00f0..4cff9ee6 100644 --- a/lite/proxy/block.go +++ b/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()) diff --git a/lite/proxy/validate_test.go b/lite/proxy/validate_test.go new file mode 100644 index 00000000..971a5e5b --- /dev/null +++ b/lite/proxy/validate_test.go @@ -0,0 +1,256 @@ +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 ( + deabBeefTxs = types.Txs{[]byte("DE"), []byte("AD"), []byte("BE"), []byte("EF")} + deadBeefRipEmd160Hash = deabBeefTxs.Hash() +) + +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: &types.Header{ + Height: 11, + Time: time.Date(2018, 1, 1, 1, 1, 1, 1, time.UTC), + ValidatorsHash: []byte("Tendermint"), + }, + }, + commit: lite.Commit{Header: &types.Header{Height: 11}}, + wantErr: "Headers don't match", + }, + + { + block: &types.Block{ + Header: &types.Header{ + Height: 11, + Time: time.Date(2018, 1, 1, 1, 1, 1, 1, time.UTC), + ValidatorsHash: []byte("Tendermint"), + }, + }, + commit: lite.Commit{ + Header: &types.Header{ + Height: 11, + Time: time.Date(2018, 1, 1, 1, 1, 1, 1, time.UTC), + ValidatorsHash: []byte("Tendermint"), + }, + }, + }, + // 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: deadBeefRipEmd160Hash}, + Data: &types.Data{Txs: deabBeefTxs}, + }, + commit: lite.Commit{ + Header: &types.Header{Height: 11}, + Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}}, + }, + }, + // End Header.Data hash mismatch test + } + + assert := assert.New(t) + + for i, tt := range tests { + err := proxy.ValidateBlock(tt.block, tt.commit) + if tt.wantErr != "" { + if err == nil { + assert.FailNowf("Unexpectedly passed", "#%d", i) + } else { + assert.Contains(err.Error(), tt.wantErr, "#%d should contain the substring\n\n", i) + } + continue + } + + assert.Nil(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: &types.Header{ + Height: 11, + Time: time.Date(2018, 1, 1, 1, 1, 1, 1, time.UTC), + ValidatorsHash: []byte("Tendermint"), + }, + }, + commit: lite.Commit{Header: &types.Header{Height: 11}}, + wantErr: "Headers don't match", + }, + + { + meta: &types.BlockMeta{ + Header: &types.Header{ + Height: 11, + Time: time.Date(2018, 1, 1, 1, 1, 1, 1, time.UTC), + ValidatorsHash: []byte("Tendermint"), + }, + }, + commit: lite.Commit{ + Header: &types.Header{ + Height: 11, + Time: time.Date(2018, 1, 1, 1, 1, 1, 1, time.UTC), + ValidatorsHash: []byte("Tendermint"), + }, + }, + }, + + { + meta: &types.BlockMeta{ + Header: &types.Header{ + Height: 11, + // TODO: (@odeke-em) inquire why ValidatorsHash has to be non-blank + // for the Header to be hashed. Perhaps this is a security hole because + // an aggressor could perhaps pass in headers that don't have + // ValidatorsHash set and we won't be able to validate blocks. + ValidatorsHash: []byte("lite-test"), + // TODO: (@odeke-em) file an issue with Tendermint to get them to update + // to the latest go-wire, then no more need for this value fill to avoid + // the time zero value of less than 1970. + Time: time.Date(2018, 1, 1, 1, 1, 1, 1, time.UTC), + }, + }, + commit: lite.Commit{ + Header: &types.Header{Height: 11, DataHash: deadBeefRipEmd160Hash}, + }, + wantErr: "Headers don't match", + }, + + { + meta: &types.BlockMeta{ + Header: &types.Header{ + Height: 11, DataHash: deadBeefRipEmd160Hash, + ValidatorsHash: []byte("Tendermint"), + Time: time.Date(2017, 1, 2, 1, 1, 1, 1, time.UTC), + }, + }, + commit: lite.Commit{ + Header: &types.Header{ + Height: 11, DataHash: deadBeefRipEmd160Hash, + ValidatorsHash: []byte("Tendermint"), + Time: time.Date(2017, 1, 2, 2, 1, 1, 1, time.UTC), + }, + Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}}, + }, + wantErr: "Headers don't match", + }, + + { + meta: &types.BlockMeta{ + Header: &types.Header{ + Height: 11, DataHash: deadBeefRipEmd160Hash, + ValidatorsHash: []byte("Tendermint"), + Time: time.Date(2017, 1, 2, 1, 1, 1, 1, time.UTC), + }, + }, + commit: lite.Commit{ + Header: &types.Header{ + Height: 11, DataHash: deadBeefRipEmd160Hash, + ValidatorsHash: []byte("Tendermint-x"), + Time: time.Date(2017, 1, 2, 1, 1, 1, 1, time.UTC), + }, + Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}}, + }, + wantErr: "Headers don't match", + }, + // End Headers don't match test + } + + assert := assert.New(t) + + for i, tt := range tests { + err := proxy.ValidateBlockMeta(tt.meta, tt.commit) + if tt.wantErr != "" { + if err == nil { + assert.FailNowf("Unexpectedly passed", "#%d: wanted error %q", i, tt.wantErr) + } else { + assert.Contains(err.Error(), tt.wantErr, "#%d should contain the substring\n\n", i) + } + continue + } + + assert.Nil(err, "#%d: expecting a nil error", i) + } +}