-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: x/gov/client/cli: fix integer overflow in parsing int prompted v…
…alues 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
- Loading branch information
Showing
2 changed files
with
95 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
//go:build !race | ||
// +build !race | ||
|
||
// Disabled -race because the package github.com/manifoldco/[email protected] | ||
// has a data race and this code exposes it, but fixing it would require | ||
// holding up the associated change to this. | ||
|
||
package cli_test | ||
|
||
import ( | ||
"fmt" | ||
"math" | ||
"os" | ||
"strings" | ||
"testing" | ||
|
||
"github.com/chzyer/readline" | ||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/cosmos/cosmos-sdk/x/gov/client/cli" | ||
) | ||
|
||
type st struct { | ||
I int | ||
} | ||
|
||
// 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) { | ||
origStdin := readline.Stdin | ||
fin, fw := readline.NewFillableStdin(os.Stdin) | ||
readline.Stdin = fin | ||
fw.Write([]byte(overflowStr + "\n")) | ||
defer func() { | ||
readline.Stdin = origStdin | ||
}() | ||
|
||
v, err := cli.Prompt(st{}, "") | ||
assert.Equal(t, st{}, v, "expected a value of zero") | ||
require.NotNil(t, err, "expected a report of an overflow") | ||
require.Contains(t, err.Error(), "range") | ||
}) | ||
} | ||
} | ||
|
||
func TestPromptParseInteger(t *testing.T) { | ||
// Intentionally sending a value out of the range of | ||
values := []struct { | ||
in string | ||
want int | ||
}{ | ||
{fmt.Sprintf("%d", math.MinInt), math.MinInt}, | ||
{"19991", 19991}, | ||
{"991000000199", 991000000199}, | ||
} | ||
|
||
for _, tc := range values { | ||
tc := tc | ||
t.Run(tc.in, func(t *testing.T) { | ||
origStdin := readline.Stdin | ||
readline.Stdin = readline.NewCancelableStdin(strings.NewReader(tc.in + "\n")) | ||
defer func() { | ||
readline.Stdin = origStdin | ||
}() | ||
|
||
v, err := cli.Prompt(st{}, "") | ||
assert.Nil(t, err, "expected a nil error") | ||
assert.Equal(t, tc.want, v.I, "expected a value of zero") | ||
}) | ||
} | ||
} |