x/genutil: fix CollectTxs traversal logic (cosmos#6913)

* x/genutil: fix CollectTxs traversal logic

Fixes the file traversal of CollectTxs to correctly skip over
directories instead of trying to read them, failing and erroring
out.

Also while here, changed the order to perform the os read
only after the AppState UnmarshalJSON has succeeded, otherwise
an attack vector can be to purposefully request many file stats
which touches kernel resources, while  just causing failures
indefinitely.

Fixes cosmos#6788

* address comments

* Fix test by passing in blank AppState

Co-authored-by: Alexander Bezobchuk <[email protected]>
Co-authored-by: SaReN <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

x/genutil/collect.go

@@ -72,20 +72,19 @@ func GenAppStateFromConfig(cdc codec.JSONMarshaler, txEncodingConfig client.TxEn
 func CollectTxs(cdc codec.JSONMarshaler, txJSONDecoder sdk.TxDecoder, moniker, genTxsDir string,
 	genDoc tmtypes.GenesisDoc, genBalIterator types.GenesisBalancesIterator,
 ) (appGenTxs []sdk.Tx, persistentPeers string, err error) {
-
-	var fos []os.FileInfo
-	fos, err = ioutil.ReadDir(genTxsDir)
-	if err != nil {
-		return appGenTxs, persistentPeers, err
-	}
-
 	// prepare a map of all balances in genesis state to then validate
 	// against the validators addresses
 	var appState map[string]json.RawMessage
 	if err := json.Unmarshal(genDoc.AppState, &appState); err != nil {
 		return appGenTxs, persistentPeers, err
 	}
 
+	var fos []os.FileInfo
+	fos, err = ioutil.ReadDir(genTxsDir)
+	if err != nil {
+		return appGenTxs, persistentPeers, err
+	}
+
 	balancesMap := make(map[string]bankexported.GenesisBalance)
 
 	genBalIterator.IterateGenesisBalances(
@@ -100,15 +99,16 @@ func CollectTxs(cdc codec.JSONMarshaler, txJSONDecoder sdk.TxDecoder, moniker, g
 	var addressesIPs []string
 
 	for _, fo := range fos {
-		filename := filepath.Join(genTxsDir, fo.Name())
-		if !fo.IsDir() && (filepath.Ext(filename) != ".json") {
+		if fo.IsDir() {
+			continue
+		}
+		if !strings.HasSuffix(fo.Name(), ".json") {
 			continue
 		}
 
 		// get the genTx
-		var jsonRawTx []byte
-
-		if jsonRawTx, err = ioutil.ReadFile(filename); err != nil {
+		jsonRawTx, err := ioutil.ReadFile(filepath.Join(genTxsDir, fo.Name()))
+		if err != nil {
 			return appGenTxs, persistentPeers, err
 		}
 

x/genutil/collect_test.go

@@ -0,0 +1,68 @@
+package genutil_test
+
+import (
+	"encoding/json"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/gogo/protobuf/proto"
+
+	tmtypes "github.com/tendermint/tendermint/types"
+
+	"github.com/cosmos/cosmos-sdk/codec"
+	cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
+	"github.com/cosmos/cosmos-sdk/server"
+	"github.com/cosmos/cosmos-sdk/types"
+	bankexported "github.com/cosmos/cosmos-sdk/x/bank/exported"
+	"github.com/cosmos/cosmos-sdk/x/genutil"
+	gtypes "github.com/cosmos/cosmos-sdk/x/genutil/types"
+)
+
+type doNothingUnmarshalJSON struct {
+	codec.JSONMarshaler
+}
+
+func (dnj *doNothingUnmarshalJSON) UnmarshalJSON(_ []byte, _ proto.Message) error {
+	return nil
+}
+
+type doNothingIterator struct {
+	gtypes.GenesisBalancesIterator
+}
+
+func (dni *doNothingIterator) IterateGenesisBalances(_ codec.JSONMarshaler, _ map[string]json.RawMessage, _ func(bankexported.GenesisBalance) bool) {
+}
+
+// Ensures that CollectTx correctly traverses directories and won't error out on encountering
+// a directory during traversal of the first level. See issue https://github.com/cosmos/cosmos-sdk/issues/6788.
+func TestCollectTxsHandlesDirectories(t *testing.T) {
+	testDir, err := ioutil.TempDir(os.TempDir(), "testCollectTxs")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(testDir)
+
+	// 1. We'll insert a directory as the first element before JSON file.
+	subDirPath := filepath.Join(testDir, "_adir")
+	if err := os.MkdirAll(subDirPath, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	txDecoder := types.TxDecoder(func(txBytes []byte) (types.Tx, error) {
+		return nil, nil
+	})
+
+	// 2. Ensure that we don't encounter any error traversing the directory.
+	srvCtx := server.NewDefaultContext()
+	_ = srvCtx
+	cdc := codec.NewProtoCodec(cdctypes.NewInterfaceRegistry())
+	gdoc := tmtypes.GenesisDoc{AppState: []byte("{}")}
+	balItr := new(doNothingIterator)
+
+	dnc := &doNothingUnmarshalJSON{cdc}
+	if _, _, err := genutil.CollectTxs(dnc, txDecoder, "foo", testDir, gdoc, balItr); err != nil {
+		t.Fatal(err)
+	}
+}