Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Commit

Permalink
fix: x/gov/client/cli: fix integer overflow in parsing int prompted v…
Browse files Browse the repository at this point in the history
…alues (cosmos#13347)

Reported by cosmos/gosec in https://github.com/cosmos/cosmos-sdk/security/code-scanning/5730.
This change checks for errors from strconv.Atoi in which case we were
susceptible to out of range errors, this change also adds tests to
prevent regressions.

Fixes cosmos#13346
  • Loading branch information
odeke-em authored Sep 20, 2022
1 parent 79f277c commit 56a49ab
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
11 changes: 10 additions & 1 deletion x/gov/client/cli/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,16 @@ func Prompt[T any](data T, namePrefix string) (T, error) {
case reflect.String:
v.Field(i).SetString(result)
case reflect.Int:
resultInt, _ := strconv.Atoi(result)
resultInt, err := strconv.Atoi(result)
if err != nil {
return data, fmt.Errorf("invalid value for int: %w", err)
}
// If a value was successfully parsed the ranges of:
// [minInt, maxInt]
// are within the ranges of:
// [minInt64, maxInt64]
// of which on 64-bit machines, which are most common,
// int==int64
v.Field(i).SetInt(int64(resultInt))
default:
// skip other types
Expand Down
43 changes: 43 additions & 0 deletions x/gov/client/cli/prompt_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package cli_test

import (
"sync"
"testing"

"github.com/chzyer/readline"
"github.com/stretchr/testify/assert"

"github.com/cosmos/cosmos-sdk/x/gov/client/cli"
)

type st struct {
ToOverflow int
}

// On the tests running in Github actions, somehow there are races.
var globalMu sync.Mutex

// Tests that we successfully report overflows in parsing ints
// See https://github.com/cosmos/cosmos-sdk/issues/13346
func TestPromptIntegerOverflow(t *testing.T) {
// Intentionally sending a value out of the range of
intOverflowers := []string{
"-9223372036854775809",
"9223372036854775808",
"9923372036854775809",
"-9923372036854775809",
"18446744073709551616",
"-18446744073709551616",
}

for _, intOverflower := range intOverflowers {
overflowStr := intOverflower
t.Run(overflowStr, func(t *testing.T) {
readline.Stdout.Write([]byte(overflowStr + "\n"))

v, err := cli.Prompt(st{}, "")
assert.NotNil(t, err, "expected a report of an overflow")
assert.Equal(t, st{}, v, "expected a value of zero")
})
}
}

0 comments on commit 56a49ab

Please sign in to comment.