Skip to content

Commit

Permalink
fix: fix uint64->uint32 underflow issues resulting from using cosmoss…
Browse files Browse the repository at this point in the history
…dk.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
  • Loading branch information
odeke-em committed Mar 13, 2024
1 parent 3157d97 commit 3425674
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 56 deletions.
3 changes: 3 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions internal/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module github.com/peggyjv/sommelier/internal

go 1.23
28 changes: 28 additions & 0 deletions internal/utils.go
Original file line number Diff line number Diff line change
@@ -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
}
23 changes: 11 additions & 12 deletions x/auction/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 3 additions & 4 deletions x/auction/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
43 changes: 21 additions & 22 deletions x/axelarcork/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -477,15 +476,15 @@ 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
}

contract := args[1]

req := &types.QueryWinningAxelarCorkRequest{
ChainId: chainID.Uint64(),
ChainId: chainID,
ContractAddress: contract,
}

Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 3425674

Please sign in to comment.