Merge PR #2800: Limit total number of signatures per transaction

* Limit total number of signatures per transaction
* Fail if limit is exceeded
* Loop over all sigs and count subkeys
* No need for a type switch, adopt early return
* Test rejection logic
* Mention sigs limit

PENDING.md

@@ -13,6 +13,7 @@ BREAKING CHANGES
 
 * SDK
   * [\#2752](https://github.com/cosmos/cosmos-sdk/pull/2752) Don't hardcode bondable denom.
+  * [\#2019](https://github.com/cosmos/cosmos-sdk/issues/2019) Cap total number of signatures. Current per-transaction limit is 7, and if that is exceeded transaction is rejected.
 
 * Tendermint
 

docs/reference/baseapp.md

@@ -2,8 +2,7 @@
 
 `baseApp` requires stores to be mounted via capabilities keys - handlers can only access stores they're given the key to. The `baseApp` ensures all stores are properly loaded, cached, and committed. One mounted store is considered the "main" - it holds the latest block header, from which we can find and load the most recent state.
 
-`baseApp` distinguishes between two handler types - the `AnteHandler` and the `MsgHandler`. The former is a global validity check (checking nonces, sigs and sufficient balances to pay fees,
-e.g. things that apply to all transaction from all modules), the later is the full state transition function.
+`baseApp` distinguishes between two handler types: `AnteHandler` and `MsgHandler`. Whilst the former is a global validity check that applies to all transactions from all modules, i.e. it checks nonces and whether balances are sufficient to pay fees, validates signatures and ensures that transactions don't carry too many signatures, the latter is the full state transition function.
 During CheckTx the state transition function is only applied to the checkTxState and should return
 before any expensive state transitions are run (this is up to each developer). It also needs to return the estimated
 gas cost.

types/errors.go

@@ -57,6 +57,7 @@ const (
 	CodeOutOfGas          CodeType = 12
 	CodeMemoTooLarge      CodeType = 13
 	CodeInsufficientFee   CodeType = 14
+	CodeTooManySignatures CodeType = 15
 
 	// CodespaceRoot is a codespace for error codes in this file only.
 	// Notice that 0 is an "unset" codespace, which can be overridden with
@@ -103,6 +104,8 @@ func CodeToDefaultMsg(code CodeType) string {
 		return "memo too large"
 	case CodeInsufficientFee:
 		return "insufficient fee"
+	case CodeTooManySignatures:
+		return "maximum numer of signatures exceeded"
 	default:
 		return unknownCodeMsg(code)
 	}
@@ -155,6 +158,9 @@ func ErrMemoTooLarge(msg string) Error {
 func ErrInsufficientFee(msg string) Error {
 	return newErrorWithRootCodespace(CodeInsufficientFee, msg)
 }
+func ErrTooManySignatures(msg string) Error {
+	return newErrorWithRootCodespace(CodeTooManySignatures, msg)
+}
 
 //----------------------------------------
 // Error & sdkError

x/auth/ante.go

@@ -4,9 +4,11 @@ import (
 	"bytes"
 	"encoding/hex"
 	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/crypto/ed25519"
+	"github.com/tendermint/tendermint/crypto/multisig"
 	"github.com/tendermint/tendermint/crypto/secp256k1"
 )
 
@@ -17,6 +19,8 @@ const (
 	maxMemoCharacters           = 100
 	// how much gas = 1 atom
 	gasPerUnitCost = 1000
+	// max total number of sigs per tx
+	txSigLimit = 7
 )
 
 // NewAnteHandler returns an AnteHandler that checks
@@ -74,6 +78,16 @@ func NewAnteHandler(am AccountKeeper, fck FeeCollectionKeeper) sdk.AnteHandler {
 		stdSigs := stdTx.GetSignatures() // When simulating, this would just be a 0-length slice.
 		signerAddrs := stdTx.GetSigners()
 
+		sigCount := 0
+		for i := 0; i < len(stdSigs); i++ {
+			sigCount += countSubKeys(stdSigs[i].PubKey)
+			if sigCount > txSigLimit {
+				return newCtx, sdk.ErrTooManySignatures(fmt.Sprintf(
+					"signatures: %d, limit: %d", sigCount, txSigLimit),
+				).Result(), true
+			}
+		}
+
 		// create the list of all sign bytes
 		signBytesList := getSignBytesList(newCtx.ChainID(), stdTx, stdSigs)
 		signerAccs, res := getSignerAccs(newCtx, am, signerAddrs)
@@ -308,3 +322,15 @@ func getSignBytesList(chainID string, stdTx StdTx, stdSigs []StdSignature) (sign
 	}
 	return
 }
+
+func countSubKeys(pub crypto.PubKey) int {
+	v, ok := pub.(*multisig.PubKeyMultisigThreshold)
+	if !ok {
+		return 1
+	}
+	nkeys := 0
+	for _, subkey := range v.PubKeys {
+		nkeys += countSubKeys(subkey)
+	}
+	return nkeys
+}

x/auth/ante_test.go

@@ -10,6 +10,7 @@ import (
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/crypto/ed25519"
+	"github.com/tendermint/tendermint/crypto/multisig"
 	"github.com/tendermint/tendermint/crypto/secp256k1"
 	"github.com/tendermint/tendermint/libs/log"
 )
@@ -714,3 +715,77 @@ func TestAdjustFeesByGas(t *testing.T) {
 		})
 	}
 }
+
+func TestCountSubkeys(t *testing.T) {
+	genPubKeys := func(n int) []crypto.PubKey {
+		var ret []crypto.PubKey
+		for i := 0; i < n; i++ {
+			ret = append(ret, secp256k1.GenPrivKey().PubKey())
+		}
+		return ret
+	}
+	genMultiKey := func(n, k int, keysGen func(n int) []crypto.PubKey) crypto.PubKey {
+		return multisig.NewPubKeyMultisigThreshold(k, keysGen(n))
+	}
+	type args struct {
+		pub crypto.PubKey
+	}
+	mkey := genMultiKey(5, 4, genPubKeys)
+	mkeyType := mkey.(*multisig.PubKeyMultisigThreshold)
+	mkeyType.PubKeys = append(mkeyType.PubKeys, genMultiKey(6, 5, genPubKeys))
+	tests := []struct {
+		name string
+		args args
+		want int
+	}{
+		{"single key", args{secp256k1.GenPrivKey().PubKey()}, 1},
+		{"multi sig key", args{genMultiKey(5, 4, genPubKeys)}, 5},
+		{"multi multi sig", args{mkey}, 11},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(T *testing.T) {
+			require.Equal(t, tt.want, countSubKeys(tt.args.pub))
+		})
+	}
+}
+
+func TestAnteHandlerSigLimitExceeded(t *testing.T) {
+	// setup
+	ms, capKey, capKey2 := setupMultiStore()
+	cdc := codec.New()
+	RegisterBaseAccount(cdc)
+	mapper := NewAccountKeeper(cdc, capKey, ProtoBaseAccount)
+	feeCollector := NewFeeCollectionKeeper(cdc, capKey2)
+	anteHandler := NewAnteHandler(mapper, feeCollector)
+	ctx := sdk.NewContext(ms, abci.Header{ChainID: "mychainid"}, false, log.NewNopLogger())
+	ctx = ctx.WithBlockHeight(1)
+
+	// keys and addresses
+	priv1, addr1 := privAndAddr()
+	priv2, addr2 := privAndAddr()
+	priv3, addr3 := privAndAddr()
+	priv4, addr4 := privAndAddr()
+	priv5, addr5 := privAndAddr()
+	priv6, addr6 := privAndAddr()
+	priv7, addr7 := privAndAddr()
+	priv8, addr8 := privAndAddr()
+
+	// set the accounts
+	acc1 := mapper.NewAccountWithAddress(ctx, addr1)
+	acc1.SetCoins(newCoins())
+	mapper.SetAccount(ctx, acc1)
+	acc2 := mapper.NewAccountWithAddress(ctx, addr2)
+	acc2.SetCoins(newCoins())
+	mapper.SetAccount(ctx, acc2)
+
+	var tx sdk.Tx
+	msg := newTestMsg(addr1, addr2, addr3, addr4, addr5, addr6, addr7, addr8)
+	msgs := []sdk.Msg{msg}
+	fee := newStdFee()
+
+	// test rejection logic
+	privs, accnums, seqs := []crypto.PrivKey{priv1, priv2, priv3, priv4, priv5, priv6, priv7, priv8},
+		[]int64{0, 0, 0, 0, 0, 0, 0, 0}, []int64{0, 0, 0, 0, 0, 0, 0, 0}
+	tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee)
+	checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeTooManySignatures)
+}