From 5b88eb0379f38afcb5fa149ebdc9e09f107a871a Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Tue, 20 Sep 2022 15:02:43 -0700 Subject: [PATCH] fix: x/gov/client/cli: fix integer overflow in parsing int prompted values 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 #13346 --- x/gov/client/cli/prompt.go | 11 ++++++++- x/gov/client/cli/prompt_test.go | 43 +++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 x/gov/client/cli/prompt_test.go diff --git a/x/gov/client/cli/prompt.go b/x/gov/client/cli/prompt.go index b997f5a4a8b9..faf7d352e19d 100644 --- a/x/gov/client/cli/prompt.go +++ b/x/gov/client/cli/prompt.go @@ -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 diff --git a/x/gov/client/cli/prompt_test.go b/x/gov/client/cli/prompt_test.go new file mode 100644 index 000000000000..2629cc65301b --- /dev/null +++ b/x/gov/client/cli/prompt_test.go @@ -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") + }) + } +}