From 9f66b41d90d405714df0d64fb0dd5574c3c9612d Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Wed, 13 Mar 2024 15:45:17 -0700 Subject: [PATCH] fix: fix uint64->uint32 underflow issues resulting from using cosmossdk.io/math This fixes potential security issues resulting from extraneous parsing that used cosmossdk.math.ParseUint which uses math/big.Int which is a big integer package and can allow uint64 in places where uint32 is used and there were underflow checks. The fix for this change was to simply use strconv.ParseUint(s, 10, BIT_SIZE) where BIT_SIZE is any of 32 or 64 bits for uint32 and uint64 respectively, and by extracting the shared code to an internal package. Fixes #292 --- go.mod | 3 +++ internal/go.mod | 3 +++ internal/utils.go | 28 +++++++++++++++++++++ x/auction/client/cli/query.go | 23 ++++++++--------- x/auction/client/cli/tx.go | 7 +++--- x/axelarcork/client/cli/query.go | 43 ++++++++++++++++---------------- x/axelarcork/client/cli/tx.go | 29 +++++++++++---------- x/cork/client/cli/query.go | 6 ++--- 8 files changed, 86 insertions(+), 56 deletions(-) create mode 100644 internal/go.mod create mode 100644 internal/utils.go diff --git a/go.mod b/go.mod index bb44658d..3638354e 100644 --- a/go.mod +++ b/go.mod @@ -140,6 +140,7 @@ require ( github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.1.0-rc2 // indirect github.com/opencontainers/runc v1.1.5 // indirect + github.com/peggyjv/sommelier/internal v0.0.0-00010101000000-000000000000 // indirect github.com/pelletier/go-toml v1.9.5 // indirect github.com/pelletier/go-toml/v2 v2.0.7 // indirect github.com/petermattis/goid v0.0.0-20230317030725-371a4b8eda08 // indirect @@ -195,6 +196,8 @@ require ( sigs.k8s.io/yaml v1.3.0 // indirect ) +replace github.com/peggyjv/sommelier/internal => ./internal + replace github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 replace github.com/99designs/keyring => github.com/cosmos/keyring v1.1.7-0.20210622111912-ef00f8ac3d76 diff --git a/internal/go.mod b/internal/go.mod new file mode 100644 index 00000000..34e120d9 --- /dev/null +++ b/internal/go.mod @@ -0,0 +1,3 @@ +module github.com/peggyjv/sommelier/internal + +go 1.19 diff --git a/internal/utils.go b/internal/utils.go new file mode 100644 index 00000000..b1728db5 --- /dev/null +++ b/internal/utils.go @@ -0,0 +1,28 @@ +package internal + +import ( + "fmt" + "strconv" +) + +// ParseUint64 parses s as a decimal uint64 value. +func ParseUint64(s string) (uint64, error) { + return strconv.ParseUint(s, 10, 64) +} + +// ParseUint32 parses s as a decimal uint32 value. +func ParseUint32(s string) (uint32, error) { + // In response to https://github.com/PeggyJV/sommelier/issues/292 + // let's use strconv.ParseUint to properly catch range errors and + // avoid underflows. + u, err := strconv.ParseUint(s, 10, 32) + if err != nil { + return 0, err + } + // Bullet-proof check to ensure no underflows (even though we already have range checks) + u32 := uint32(u) + if g := uint64(u32); g != u { + return 0, fmt.Errorf("parseuint32 underflow detected: got %d, want %d", g, u) + } + return u32, nil +} diff --git a/x/auction/client/cli/query.go b/x/auction/client/cli/query.go index a2a4f0fc..d5258059 100644 --- a/x/auction/client/cli/query.go +++ b/x/auction/client/cli/query.go @@ -3,11 +3,10 @@ package cli import ( "fmt" - "cosmossdk.io/math" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/peggyjv/sommelier/v7/x/auction/types" + "github.com/peggyjv/sommelier/internal" "github.com/spf13/cobra" ) @@ -80,14 +79,14 @@ func queryActiveAuction() *cobra.Command { return err } - auctionID, err := math.ParseUint(args[0]) + auctionID, err := internal.ParseUint32(args[0]) if err != nil { return err } queryClient := types.NewQueryClient(ctx) req := &types.QueryActiveAuctionRequest{ - AuctionId: uint32(auctionID.Uint64()), + AuctionId: auctionID, } res, err := queryClient.QueryActiveAuction(cmd.Context(), req) @@ -116,14 +115,14 @@ func queryEndedAuction() *cobra.Command { return err } - auctionID, err := math.ParseUint(args[0]) + auctionID, err := internal.ParseUint32(args[0]) if err != nil { return err } queryClient := types.NewQueryClient(ctx) req := &types.QueryEndedAuctionRequest{ - AuctionId: uint32(auctionID.Uint64()), + AuctionId: auctionID, } res, err := queryClient.QueryEndedAuction(cmd.Context(), req) @@ -288,20 +287,20 @@ func queryBid() *cobra.Command { return err } - auctionID, err := math.ParseUint(args[0]) + auctionID, err := internal.ParseUint32(args[0]) if err != nil { return err } - bidID, err := math.ParseUint(args[0]) + bidID, err := internal.ParseUint64(args[0]) if err != nil { return err } queryClient := types.NewQueryClient(ctx) req := &types.QueryBidRequest{ - AuctionId: uint32(auctionID.Uint64()), - BidId: bidID.Uint64(), + AuctionId: auctionID, + BidId: bidID, } res, err := queryClient.QueryBid(cmd.Context(), req) @@ -330,14 +329,14 @@ func queryBidsByAuction() *cobra.Command { return err } - auctionID, err := math.ParseUint(args[0]) + auctionID, err := internal.ParseUint32(args[0]) if err != nil { return err } queryClient := types.NewQueryClient(ctx) req := &types.QueryBidsByAuctionRequest{ - AuctionId: uint32(auctionID.Uint64()), + AuctionId: auctionID, } res, err := queryClient.QueryBidsByAuction(cmd.Context(), req) diff --git a/x/auction/client/cli/tx.go b/x/auction/client/cli/tx.go index f4890e1c..a60b1baf 100644 --- a/x/auction/client/cli/tx.go +++ b/x/auction/client/cli/tx.go @@ -5,14 +5,13 @@ import ( "os" "strings" - "cosmossdk.io/math" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/client/tx" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/version" govtypesv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" + "github.com/peggyjv/sommelier/internal" types "github.com/peggyjv/sommelier/v7/x/auction/types" "github.com/spf13/cobra" ) @@ -121,7 +120,7 @@ $ %s tx auction submit-bid 1 10000usomm 50000gravity0xdac17f958d2ee523a220620699 return err } - auctionID, err := math.ParseUint(args[0]) + auctionID, err := internal.ParseUint32(args[0]) if err != nil { return err } @@ -141,7 +140,7 @@ $ %s tx auction submit-bid 1 10000usomm 50000gravity0xdac17f958d2ee523a220620699 return fmt.Errorf("must include `--from` flag") } - msg, err := types.NewMsgSubmitBidRequest(uint32(auctionID.Uint64()), maxBidInUsomm, saleTokenMinimumAmount, bidder) + msg, err := types.NewMsgSubmitBidRequest(auctionID, maxBidInUsomm, saleTokenMinimumAmount, bidder) if err != nil { return err } diff --git a/x/axelarcork/client/cli/query.go b/x/axelarcork/client/cli/query.go index 94e8781f..6103b2bf 100644 --- a/x/axelarcork/client/cli/query.go +++ b/x/axelarcork/client/cli/query.go @@ -3,10 +3,9 @@ package cli import ( "fmt" - "cosmossdk.io/math" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" + "github.com/peggyjv/sommelier/internal" "github.com/peggyjv/sommelier/v7/x/axelarcork/types" "github.com/spf13/cobra" ) @@ -112,13 +111,13 @@ func queryCellarIDsByChainID() *cobra.Command { return err } - chainID, err := math.ParseUint(args[0]) + chainID, err := internal.ParseUint64(args[0]) if err != nil { return err } req := &types.QueryCellarIDsByChainIDRequest{ - ChainId: chainID.Uint64(), + ChainId: chainID, } queryClient := types.NewQueryClient(ctx) @@ -151,13 +150,13 @@ func queryScheduledCorks() *cobra.Command { queryClient := types.NewQueryClient(ctx) - chainID, err := math.ParseUint(args[0]) + chainID, err := internal.ParseUint64(args[0]) if err != nil { return err } req := &types.QueryScheduledCorksRequest{ - ChainId: chainID.Uint64(), + ChainId: chainID, } res, err := queryClient.QueryScheduledCorks(cmd.Context(), req) @@ -186,20 +185,20 @@ func queryScheduledCorksByBlockHeight() *cobra.Command { return err } - height, err := math.ParseUint(args[0]) + height, err := internal.ParseUint64(args[0]) if err != nil { return err } queryClient := types.NewQueryClient(ctx) - chainID, err := math.ParseUint(args[0]) + chainID, err := internal.ParseUint64(args[0]) if err != nil { return err } req := &types.QueryScheduledCorksByBlockHeightRequest{ - BlockHeight: height.Uint64(), - ChainId: chainID.Uint64(), + BlockHeight: height, + ChainId: chainID, } res, err := queryClient.QueryScheduledCorksByBlockHeight(cmd.Context(), req) @@ -229,13 +228,13 @@ func queryScheduledBlockHeights() *cobra.Command { } queryClient := types.NewQueryClient(ctx) - chainID, err := math.ParseUint(args[0]) + chainID, err := internal.ParseUint64(args[0]) if err != nil { return err } req := &types.QueryScheduledBlockHeightsRequest{ - ChainId: chainID.Uint64(), + ChainId: chainID, } res, err := queryClient.QueryScheduledBlockHeights(cmd.Context(), req) @@ -272,14 +271,14 @@ func queryScheduledCorksByID() *cobra.Command { } queryClient := types.NewQueryClient(ctx) - chainID, err := math.ParseUint(args[0]) + chainID, err := internal.ParseUint64(args[0]) if err != nil { return err } req := &types.QueryScheduledCorksByIDRequest{ Id: id, - ChainId: chainID.Uint64(), + ChainId: chainID, } res, err := queryClient.QueryScheduledCorksByID(cmd.Context(), req) if err != nil { @@ -314,14 +313,14 @@ func queryCorkResult() *cobra.Command { } queryClient := types.NewQueryClient(ctx) - chainID, err := math.ParseUint(args[1]) + chainID, err := internal.ParseUint64(args[1]) if err != nil { return err } req := &types.QueryCorkResultRequest{ Id: corkID, - ChainId: chainID.Uint64(), + ChainId: chainID, } res, err := queryClient.QueryCorkResult(cmd.Context(), req) @@ -351,13 +350,13 @@ func queryCorkResults() *cobra.Command { } queryClient := types.NewQueryClient(ctx) - chainID, err := math.ParseUint(args[0]) + chainID, err := internal.ParseUint64(args[0]) if err != nil { return err } req := &types.QueryCorkResultsRequest{ - ChainId: chainID.Uint64(), + ChainId: chainID, } res, err := queryClient.QueryCorkResults(cmd.Context(), req) @@ -477,7 +476,7 @@ func queryWinningAxelarCork() *cobra.Command { queryClient := types.NewQueryClient(ctx) - chainID, err := math.ParseUint(args[0]) + chainID, err := internal.ParseUint64(args[0]) if err != nil { return err } @@ -485,7 +484,7 @@ func queryWinningAxelarCork() *cobra.Command { contract := args[1] req := &types.QueryWinningAxelarCorkRequest{ - ChainId: chainID.Uint64(), + ChainId: chainID, ContractAddress: contract, } @@ -517,13 +516,13 @@ func queryWinningAxelarCorks() *cobra.Command { queryClient := types.NewQueryClient(ctx) - chainID, err := math.ParseUint(args[0]) + chainID, err := internal.ParseUint64(args[0]) if err != nil { return err } req := &types.QueryWinningAxelarCorksRequest{ - ChainId: chainID.Uint64(), + ChainId: chainID, } res, err := queryClient.QueryWinningAxelarCorks(cmd.Context(), req) diff --git a/x/axelarcork/client/cli/tx.go b/x/axelarcork/client/cli/tx.go index df7fd59c..ccd1491a 100644 --- a/x/axelarcork/client/cli/tx.go +++ b/x/axelarcork/client/cli/tx.go @@ -5,8 +5,6 @@ import ( "os" "strings" - "cosmossdk.io/math" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/client/tx" @@ -14,6 +12,7 @@ import ( "github.com/cosmos/cosmos-sdk/version" govtypesv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" "github.com/ethereum/go-ethereum/common" + "github.com/peggyjv/sommelier/internal" types "github.com/peggyjv/sommelier/v7/x/axelarcork/types" pubsubtypes "github.com/peggyjv/sommelier/v7/x/pubsub/types" "github.com/spf13/cobra" @@ -56,7 +55,7 @@ func CmdScheduleAxelarCork() *cobra.Command { from := clientCtx.GetFromAddress() - chainID, err := math.ParseUint(args[0]) + chainID, err := internal.ParseUint64(args[0]) if err != nil { return err } @@ -66,7 +65,7 @@ func CmdScheduleAxelarCork() *cobra.Command { return fmt.Errorf("contract address %s is invalid", contractAddr) } - blockHeight, err := math.ParseUint(args[2]) + blockHeight, err := internal.ParseUint64(args[2]) if err != nil { return err } @@ -76,11 +75,11 @@ func CmdScheduleAxelarCork() *cobra.Command { scheduleCorkMsg := types.MsgScheduleAxelarCorkRequest{ Cork: &types.AxelarCork{ EncodedContractCall: contractCallBz, - ChainId: chainID.Uint64(), + ChainId: chainID, TargetContractAddress: contractAddr, }, - ChainId: chainID.Uint64(), - BlockHeight: blockHeight.Uint64(), + ChainId: chainID, + BlockHeight: blockHeight, Signer: from.String(), } if err := scheduleCorkMsg.ValidateBasic(); err != nil { @@ -109,7 +108,7 @@ func CmdRelayAxelarCork() *cobra.Command { from := clientCtx.GetFromAddress() - chainID, err := math.ParseUint(args[0]) + chainID, err := internal.ParseUint64(args[0]) if err != nil { return err } @@ -124,7 +123,7 @@ func CmdRelayAxelarCork() *cobra.Command { return err } - fee, err := math.ParseUint(args[3]) + fee, err := internal.ParseUint64(args[3]) if err != nil { return err } @@ -132,8 +131,8 @@ func CmdRelayAxelarCork() *cobra.Command { relayCorkMsg := types.MsgRelayAxelarCorkRequest{ Signer: from.String(), Token: token, - Fee: fee.Uint64(), - ChainId: chainID.Uint64(), + Fee: fee, + ChainId: chainID, TargetContractAddress: contractAddr, } if err := relayCorkMsg.ValidateBasic(); err != nil { @@ -162,7 +161,7 @@ func CmdRelayAxelarProxyUpgrade() *cobra.Command { from := clientCtx.GetFromAddress() - chainID, err := math.ParseUint(args[0]) + chainID, err := internal.ParseUint64(args[0]) if err != nil { return err } @@ -172,7 +171,7 @@ func CmdRelayAxelarProxyUpgrade() *cobra.Command { return err } - fee, err := math.ParseUint(args[2]) + fee, err := internal.ParseUint64(args[2]) if err != nil { return err } @@ -180,8 +179,8 @@ func CmdRelayAxelarProxyUpgrade() *cobra.Command { relayProxyUpgradeMsg := types.MsgRelayAxelarProxyUpgradeRequest{ Signer: from.String(), Token: token, - Fee: fee.Uint64(), - ChainId: chainID.Uint64(), + Fee: fee, + ChainId: chainID, } if err := relayProxyUpgradeMsg.ValidateBasic(); err != nil { return err diff --git a/x/cork/client/cli/query.go b/x/cork/client/cli/query.go index 34b8bdae..b09e2057 100644 --- a/x/cork/client/cli/query.go +++ b/x/cork/client/cli/query.go @@ -3,9 +3,9 @@ package cli import ( "fmt" - "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" + "github.com/peggyjv/sommelier/internal" types "github.com/peggyjv/sommelier/v7/x/cork/types/v2" "github.com/spf13/cobra" ) @@ -134,14 +134,14 @@ func queryScheduledCorksByBlockHeight() *cobra.Command { return err } - height, err := math.ParseUint(args[0]) + height, err := internal.ParseUint64(args[0]) if err != nil { return err } queryClient := types.NewQueryClient(ctx) req := &types.QueryScheduledCorksByBlockHeightRequest{ - BlockHeight: height.Uint64(), + BlockHeight: height, } res, err := queryClient.QueryScheduledCorksByBlockHeight(cmd.Context(), req)