BaseApp Security Improvements

PENDING.md

@@ -78,6 +78,9 @@ CLI flag.
 * [\#3300] Update the spec-spec, spec file reorg, and TOC updates.
 * [\#3694] Push tagged docker images on docker hub when tag is created.
 * [\#3716] Update file permissions the client keys directory and contents to `0700`.
+* [\#3791] Ensure proper handling of invalid block maximum gas in the `BaseApp`.
+* [\#3790] Ensure a valid height is provided by `abci.RequestBeginBlock` in
+`BeginBlock`.
 
 ### Tendermint
 

baseapp/baseapp.go

@@ -287,12 +287,19 @@ func (app *BaseApp) storeConsensusParams(consensusParams *abci.ConsensusParams)
 	mainStore.Set(mainConsensusParamsKey, consensusParamsBz)
 }
 
-// getMaximumBlockGas gets the maximum gas from the consensus params.
-func (app *BaseApp) getMaximumBlockGas() (maxGas uint64) {
+// getMaximumBlockGas gets the maximum gas from the consensus params. It panics
+// when the maximum block gas is non-positive.
+func (app *BaseApp) getMaximumBlockGas() uint64 {
 	if app.consensusParams == nil || app.consensusParams.BlockSize == nil {
 		return 0
 	}
-	return uint64(app.consensusParams.BlockSize.MaxGas)
+
+	maxGas := app.consensusParams.BlockSize.MaxGas
+	if maxGas < 0 {
+		return 0
+	}
+
+	return uint64(maxGas)
 }
 
 // ----------------------------------------------------------------------------
@@ -510,6 +517,19 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res
 	}
 }
 
+func (app *BaseApp) validateHeight(req abci.RequestBeginBlock) error {
+	if req.Header.Height < 0 {
+		return fmt.Errorf("invalid height: %d", req.Header.Height)
+	}
+
+	prevHeight := app.LastBlockHeight()
+	if prevHeight != 0 && req.Header.Height != prevHeight+1 {
+		return fmt.Errorf("invalid height: %d; expected: %d", req.Header.Height, prevHeight+1)
+	}
+
+	return nil
+}
+
 // BeginBlock implements the ABCI application interface.
 func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeginBlock) {
 	if app.cms.TracingEnabled() {
@@ -518,6 +538,10 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
 		))
 	}
 
+	if err := app.validateHeight(req); err != nil {
+		panic(err)
+	}
+
 	// Initialize the DeliverTx state. If this is the first block, it should
 	// already be initialized in InitChain. Otherwise app.deliverState will be
 	// nil, since it is reset on Commit.

baseapp/baseapp_test.go

@@ -296,6 +296,7 @@ func TestInitChainer(t *testing.T) {
 	// stores are mounted and private members are set - sealing baseapp
 	err := app.LoadLatestVersion(capKey) // needed to make stores non-nil
 	require.Nil(t, err)
+	require.Equal(t, int64(0), app.LastBlockHeight())
 
 	app.InitChain(abci.RequestInitChain{AppStateBytes: []byte("{}"), ChainId: "test-chain-id"}) // must have valid JSON genesis file, even if empty
 
@@ -308,6 +309,7 @@ func TestInitChainer(t *testing.T) {
 
 	app.Commit()
 	res = app.Query(query)
+	require.Equal(t, int64(1), app.LastBlockHeight())
 	require.Equal(t, value, res.Value)
 
 	// reload app
@@ -316,14 +318,17 @@ func TestInitChainer(t *testing.T) {
 	app.MountStores(capKey, capKey2)
 	err = app.LoadLatestVersion(capKey) // needed to make stores non-nil
 	require.Nil(t, err)
+	require.Equal(t, int64(1), app.LastBlockHeight())
 
 	// ensure we can still query after reloading
 	res = app.Query(query)
 	require.Equal(t, value, res.Value)
 
 	// commit and ensure we can still query
-	app.BeginBlock(abci.RequestBeginBlock{})
+	header := abci.Header{Height: app.LastBlockHeight() + 1}
+	app.BeginBlock(abci.RequestBeginBlock{Header: header})
 	app.Commit()
+
 	res = app.Query(query)
 	require.Equal(t, value, res.Value)
 }
@@ -514,7 +519,6 @@ func TestCheckTx(t *testing.T) {
 	app := setupBaseApp(t, anteOpt, routerOpt)
 
 	nTxs := int64(5)
-
 	app.InitChain(abci.RequestInitChain{})
 
 	// Create same codec used in txDecoder
@@ -567,16 +571,22 @@ func TestDeliverTx(t *testing.T) {
 
 	nBlocks := 3
 	txPerHeight := 5
+
 	for blockN := 0; blockN < nBlocks; blockN++ {
-		app.BeginBlock(abci.RequestBeginBlock{})
+		header := abci.Header{Height: int64(blockN) + 1}
+		app.BeginBlock(abci.RequestBeginBlock{Header: header})
+
 		for i := 0; i < txPerHeight; i++ {
 			counter := int64(blockN*txPerHeight + i)
 			tx := newTxCounter(counter, counter)
+
 			txBytes, err := codec.MarshalBinaryLengthPrefixed(tx)
 			require.NoError(t, err)
+
 			res := app.DeliverTx(txBytes)
 			require.True(t, res.IsOK(), fmt.Sprintf("%v", res))
 		}
+
 		app.EndBlock(abci.RequestEndBlock{})
 		app.Commit()
 	}
@@ -692,7 +702,8 @@ func TestSimulateTx(t *testing.T) {
 	nBlocks := 3
 	for blockN := 0; blockN < nBlocks; blockN++ {
 		count := int64(blockN + 1)
-		app.BeginBlock(abci.RequestBeginBlock{})
+		header := abci.Header{Height: count}
+		app.BeginBlock(abci.RequestBeginBlock{Header: header})
 
 		tx := newTxCounter(count, count)
 		txBytes, err := cdc.MarshalBinaryLengthPrefixed(tx)
@@ -1216,3 +1227,16 @@ func TestP2PQuery(t *testing.T) {
 	res = app.Query(idQuery)
 	require.Equal(t, uint32(4), res.Code)
 }
+
+func TestGetMaximumBlockGas(t *testing.T) {
+	app := setupBaseApp(t)
+
+	app.setConsensusParams(&abci.ConsensusParams{BlockSize: &abci.BlockSizeParams{MaxGas: 0}})
+	require.Equal(t, uint64(0), app.getMaximumBlockGas())
+
+	app.setConsensusParams(&abci.ConsensusParams{BlockSize: &abci.BlockSizeParams{MaxGas: 5000000}})
+	require.Equal(t, uint64(5000000), app.getMaximumBlockGas())
+
+	app.setConsensusParams(&abci.ConsensusParams{BlockSize: &abci.BlockSizeParams{MaxGas: -5000000}})
+	require.Equal(t, uint64(0), app.getMaximumBlockGas())
+}