CVE-2021-41135: Improper Check for Unusual or Exceptional Conditions (CWE-754)
value: The Cosmos-SDK is a framework for building blockchain applications in Golang. Affected versions of the SDK were vulnerable to a consensus halt due to non-deterministic behaviour in a ValidateBasic method in the x/authz module. The MsgGrant of the x/authz module contains a Grant field which includes a user-defined expiration time for when the authorization grant expires. In Grant.ValidateBasic(), that time is compared to the node’s local clock time. Any chain running an affected version of the SDK with the authz module enabled could be halted by anyone with the ability to send transactions on that chain. Recovery would require applying the patch and rolling back the latest block. Users are advised to update to version 0.44.2.
CWE description: The product does not check or incorrectly checks for unusual or exceptional conditions that are not expected to occur frequently during day to day operation of the product.
CVSS v3 base score: 6.5
Published: 2021-10-20T18:15Z
Repository: https://github.com/cosmos/cosmos-sdk
Language: Go
Files changed: x/authz/authorization_grant.go, x/authz/authorization_grant_test.go, x/authz/client/testutil/tx.go, x/authz/keeper/msg_server.go, x/authz/msgs_test.go
Commit date: 2021-10-12 10:40:49 -0400
Commit message: Merge pull request from GHSA-2p6r-37p9-89p2

* test: adding authz grant tests

* fix TestCLITxGrantAuthorization/Invalid_expiration_time test case

* comment out the test

* reenable test
Diff: diff --git a/x/authz/authorization_grant.go b/x/authz/authorization_grant.go
index f5ebf8797..a873499b6 100644
--- a/x/authz/authorization_grant.go
+++ b/x/authz/authorization_grant.go
@@ -8,11 +8,15 @@ import (
 	cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 // NewGrant returns new Grant
-func NewGrant(a Authorization, expiration time.Time) (Grant, error) {
+func NewGrant( /*blockTime time.Time, */ a Authorization, expiration time.Time) (Grant, error) {
+	// TODO: add this for 0.45
+	// if !expiration.After(blockTime) {
+	// 	return Grant{}, sdkerrors.ErrInvalidRequest.Wrapf("expiration must be after the current block time (%v), got %v", blockTime.Format(time.RFC3339), expiration.Format(time.RFC3339))
+	// }
 	g := Grant{
 		Expiration: expiration,
 	}
 	msg, ok := a.(proto.Message)
 	if !ok {
@@ -49,14 +53,10 @@ func (g Grant) GetAuthorization() Authorization {
 	}
 	return a
 }
 
 func (g Grant) ValidateBasic() error {
-	if g.Expiration.Unix() < time.Now().Unix() {
-		return sdkerrors.Wrap(ErrInvalidExpirationTime, "Time can't be in the past")
-	}
-
 	av := g.Authorization.GetCachedValue()
 	a, ok := av.(Authorization)
 	if !ok {
 		return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", (Authorization)(nil), av)
 	}
diff --git a/x/authz/authorization_grant_test.go b/x/authz/authorization_grant_test.go
new file mode 100644
index 000000000..9f9f00108
--- /dev/null
+++ b/x/authz/authorization_grant_test.go
@@ -0,0 +1,44 @@
+package authz
+
+import (
+	"testing"
+	"time"
+
+	// banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	"github.com/stretchr/testify/require"
+)
+
+func expecError(r *require.Assertions, expected string, received error) {
+	if expected == "" {
+		r.NoError(received)
+	} else {
+		r.Error(received)
+		r.Contains(received.Error(), expected)
+	}
+}
+
+func TestNewGrant(t *testing.T) {
+	// ba := banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewInt64Coin("foo", 123)))
+	a := NewGenericAuthorization("some-type")
+	var tcs = []struct {
+		title     string
+		a         Authorization
+		blockTime time.Time
+		expire    time.Time
+		err       string
+	}{
+		// {"wrong expire time (1)", a, time.Unix(10, 0), time.Unix(8, 0), "expiration must be after"},
+		// {"wrong expire time (2)", a, time.Unix(10, 0), time.Unix(10, 0), "expiration must be after"},
+		{"good expire time (1)", a, time.Unix(10, 0), time.Unix(10, 1), ""},
+		{"good expire time (2)", a, time.Unix(10, 0), time.Unix(11, 0), ""},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.title, func(t *testing.T) {
+			// _, err := NewGrant(tc.blockTime, tc.a, tc.expire)
+			_, err := NewGrant(tc.a, tc.expire)
+			expecError(require.New(t), tc.err, err)
+		})
+	}
+
+}
diff --git a/x/authz/client/testutil/tx.go b/x/authz/client/testutil/tx.go
index 10932d9bd..ac003ef13 100644
--- a/x/authz/client/testutil/tx.go
+++ b/x/authz/client/testutil/tx.go
@@ -125,15 +125,15 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			[]string{
 				grantee.String(),
 				"send",
 				fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
 				fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
-				fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
+				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, pastHour),
 			},
-			0,
-			true,
+			0xd,
+			false, // TODO: enable in v0.45
 		},
 		{
 			"fail with error invalid msg-type",
 			[]string{
 				grantee.String(),
diff --git a/x/authz/keeper/msg_server.go b/x/authz/keeper/msg_server.go
index e13b29fbd..2e5183865 100644
--- a/x/authz/keeper/msg_server.go
+++ b/x/authz/keeper/msg_server.go
@@ -8,11 +8,11 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/authz"
 )
 
 var _ authz.MsgServer = Keeper{}
 
-// GrantAuthorization implements the MsgServer.Grant method.
+// GrantAuthorization implements the MsgServer.Grant method to create a new grant.
 func (k Keeper) Grant(goCtx context.Context, msg *authz.MsgGrant) (*authz.MsgGrantResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 	grantee, err := sdk.AccAddressFromBech32(msg.Grantee)
 	if err != nil {
 		return nil, err
diff --git a/x/authz/msgs_test.go b/x/authz/msgs_test.go
index 7a41c1bef..c7b4192d3 100644
--- a/x/authz/msgs_test.go
+++ b/x/authz/msgs_test.go
@@ -78,11 +78,11 @@ func TestMsgGrantAuthorization(t *testing.T) {
 		{"nil granter address", nil, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now(), false, false},
 		{"nil grantee address", granter, nil, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now(), false, false},
 		{"nil granter and grantee address", nil, nil, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now(), false, false},
 		{"nil authorization", granter, grantee, nil, time.Now(), true, false},
 		{"valid test case", granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now().AddDate(0, 1, 0), false, true},
-		{"past time", granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now().AddDate(0, 0, -1), false, false},
+		{"past time", granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now().AddDate(0, 0, -1), false, true}, // TODO need 0.45
 	}
 	for i, tc := range tests {
 		msg, err := authz.NewMsgGrant(
 			tc.granter, tc.grantee, tc.authorization, tc.expiration,
 		)

Vulnerable code: func NewGrant(a Authorization, expiration time.Time) (Grant, error) {
	if g.Expiration.Unix() < time.Now().Unix() {
		return sdkerrors.Wrap(ErrInvalidExpirationTime, "Time can't be in the past")
	}

				fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
			0,
			true,
// GrantAuthorization implements the MsgServer.Grant method.
		{"past time", granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now().AddDate(0, 0, -1), false, false},
Fixed code: func NewGrant( /*blockTime time.Time, */ a Authorization, expiration time.Time) (Grant, error) {
	// TODO: add this for 0.45
	// if !expiration.After(blockTime) {
	// 	return Grant{}, sdkerrors.ErrInvalidRequest.Wrapf("expiration must be after the current block time (%v), got %v", blockTime.Format(time.RFC3339), expiration.Format(time.RFC3339))
	// }
package authz

import (
	"testing"
	"time"

	// banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
	"github.com/stretchr/testify/require"
)

func expecError(r *require.Assertions, expected string, received error) {
	if expected == "" {
		r.NoError(received)
	} else {
		r.Error(received)
		r.Contains(received.Error(), expected)
	}
}

func TestNewGrant(t *testing.T) {
	// ba := banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewInt64Coin("foo", 123)))
	a := NewGenericAuthorization("some-type")
	var tcs = []struct {
		title     string
		a         Authorization
		blockTime time.Time
		expire    time.Time
		err       string
	}{
		// {"wrong expire time (1)", a, time.Unix(10, 0), time.Unix(8, 0), "expiration must be after"},
		// {"wrong expire time (2)", a, time.Unix(10, 0), time.Unix(10, 0), "expiration must be after"},
		{"good expire time (1)", a, time.Unix(10, 0), time.Unix(10, 1), ""},
		{"good expire time (2)", a, time.Unix(10, 0), time.Unix(11, 0), ""},
	}

	for _, tc := range tcs {
		t.Run(tc.title, func(t *testing.T) {
			// _, err := NewGrant(tc.blockTime, tc.a, tc.expire)
			_, err := NewGrant(tc.a, tc.expire)
			expecError(require.New(t), tc.err, err)
		})
	}

}
				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
			0xd,
			false, // TODO: enable in v0.45
// GrantAuthorization implements the MsgServer.Grant method to create a new grant.
		{"past time", granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now().AddDate(0, 0, -1), false, true}, // TODO need 0.45