all: use of cosmossdk.io/math.ParseUint then cast to uint32 is unnecessary and susceptible to uint64->uint32 underflows which could be a security issue

[odeke-em]

Summary of Bug

Noticed while auditing the code in x/auction/client/cli that there is this code

sommelier/x/auction/client/cli/query.go

Lines 333 to 341 in 3157d97

	auctionID, err := math.ParseUint(args[0])
	if err != nil {
	return err
	}
	queryClient := types.NewQueryClient(ctx)
	req := &types.QueryBidsByAuctionRequest{
	AuctionId: uint32(auctionID.Uint64()),
	}

in which firstly cosmossdk.io/math.ParseUint is invoked and that returns a Uint object whose underlying structure uses a math/big.Int with arbitrary precision.

Down below to send in a uint32 into the query, we invoke uint32(u.Uint64()) of which this code will cause an underflow for example if I pass in any value greater than uint32 which can actually help me with randomization to get random bids and other values and probably even clog y'all network if there are no gas fees to even make these queries

Reproduction

package repro_test

import (
	"testing"

	"cosmossdk.io/math"
)

func TestUnderflowExhibit(t *testing.T) {
	s := "4300000000"
	auctionID, err := math.ParseUint(s)
	if err != nil {
		t.Fatal(err)
	}

	dd := uint32(auctionID.Uint64())
	if g, w := uint64(dd), auctionID.Uint64(); g != w {
		t.Fatalf("Reflexive failure uint64(uint32): got %d want %d", g, w)
	}
}

Remedy

You can trivially use strconv.ParseUint(args[0], 10, 32) which is so much simpler and will directly help all these sites and catch range errors

Location

There are 11 sites where this code pattern is used and can be fixed by extracting the fix into a helper function called parseUint32

Kindly /cc-ing @zmanian @fedekunze @jackzampolin

[odeke-em]

I've mailed out PR #293

[odeke-em]

Turns out that there were even many more instances of the erroneous code in a bunch of packages so I've updated the PR #293.