Skip to content

Commit

Permalink
fix(x/auction/client/cli): fix uint64->uint32 underflow issues
Browse files Browse the repository at this point in the history
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 PeggyJV#292
  • Loading branch information
odeke-em committed Mar 13, 2024
1 parent 3157d97 commit a882442
Showing 1 changed file with 31 additions and 12 deletions.
43 changes: 31 additions & 12 deletions x/auction/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit a882442

Please sign in to comment.