From a882442861afe25fa391c1408c0fbc48ad5ab676 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Wed, 13 Mar 2024 15:45:17 -0700 Subject: [PATCH] fix(x/auction/client/cli): fix uint64->uint32 underflow issues This fixes a potential security issue 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. Fixes #292 --- x/auction/client/cli/query.go | 43 +++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/x/auction/client/cli/query.go b/x/auction/client/cli/query.go index a2a4f0fc..e91e6609 100644 --- a/x/auction/client/cli/query.go +++ b/x/auction/client/cli/query.go @@ -2,8 +2,7 @@ package cli import ( "fmt" - - "cosmossdk.io/math" + "strconv" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" @@ -68,6 +67,26 @@ func queryParams() *cobra.Command { return cmd } +func parseUint64(s string) (uint64, error) { + return strconv.ParseUint(s, 10, 64) +} + +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 +} + func queryActiveAuction() *cobra.Command { cmd := &cobra.Command{ Use: "active-auction", @@ -80,14 +99,14 @@ func queryActiveAuction() *cobra.Command { return err } - auctionID, err := math.ParseUint(args[0]) + auctionID, err := 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 +135,14 @@ func queryEndedAuction() *cobra.Command { return err } - auctionID, err := math.ParseUint(args[0]) + auctionID, err := 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 +307,20 @@ func queryBid() *cobra.Command { return err } - auctionID, err := math.ParseUint(args[0]) + auctionID, err := parseUint32(args[0]) if err != nil { return err } - bidID, err := math.ParseUint(args[0]) + bidID, err := 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 +349,14 @@ func queryBidsByAuction() *cobra.Command { return err } - auctionID, err := math.ParseUint(args[0]) + auctionID, err := 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)