Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: x/gov/client/cli: fix integer overflow in parsing int prompted values #13347

Merged

Conversation

odeke-em
Copy link
Collaborator

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

@odeke-em odeke-em requested a review from a team as a code owner September 20, 2022 22:05
…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
@odeke-em odeke-em force-pushed the x-gov-client-cli-fix-overflows-from-code-scanning-5730 branch from 6a4dede to 5b88eb0 Compare September 20, 2022 22:18
@codecov
Copy link

codecov bot commented Sep 20, 2022

Codecov Report

Merging #13347 (5b88eb0) into main (79f277c) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #13347      +/-   ##
==========================================
- Coverage   54.90%   54.86%   -0.05%     
==========================================
  Files         658      652       -6     
  Lines       56117    55859     -258     
==========================================
- Hits        30810    30645     -165     
+ Misses      22818    22733      -85     
+ Partials     2489     2481       -8     
Impacted Files Coverage Δ
x/gov/client/cli/prompt.go 11.49% <0.00%> (+11.49%) ⬆️
tx/textual/valuerenderer/dec.go
tx/textual/valuerenderer/coins.go
tx/textual/valuerenderer/valuerenderer.go
tx/textual/valuerenderer/bytes.go
tx/textual/valuerenderer/timestamp.go
tx/textual/valuerenderer/int.go
x/group/keeper/keeper.go 56.64% <0.00%> (+0.39%) ⬆️
x/staking/simulation/operations.go 75.91% <0.00%> (+1.37%) ⬆️
x/distribution/simulation/operations.go 90.32% <0.00%> (+9.67%) ⬆️
... and 1 more

@odeke-em odeke-em enabled auto-merge (squash) September 20, 2022 23:07
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@odeke-em odeke-em merged commit 56a49ab into main Sep 20, 2022
@odeke-em odeke-em deleted the x-gov-client-cli-fix-overflows-from-code-scanning-5730 branch September 20, 2022 23:26
x/gov/client/cli/prompt_test.go Show resolved Hide resolved
x/gov/client/cli/prompt_test.go Show resolved Hide resolved
x/gov/client/cli/prompt_test.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x/gov/client/cli: integer overflow from ignoring output of strconv.Atoi
3 participants