Skip to content

fix: address cosmos-gosec lint issues#1153

Merged
lumtis merged 24 commits intodevelopfrom
gosec-cosmos
Oct 2, 2023
Merged

fix: address cosmos-gosec lint issues#1153
lumtis merged 24 commits intodevelopfrom
gosec-cosmos

Conversation

@lumtis
Copy link
Contributor

@lumtis lumtis commented Sep 20, 2023

Description

Addresses the issues in Cosmos Gosec and use #nosec when no concern.

There are many #nosec added because the linter has no heuristic: will still raise potential overflow even if the range has been verified for the variable previously.

I think this is fine, using #nosec oblige us to justify when using potentially dangerous integer conversion

Closes: #1132

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

Checklist:

  • I have added unit tests that prove my fix feature works

@lumtis lumtis self-assigned this Sep 20, 2023
@github-actions
Copy link

github-actions bot commented Sep 20, 2023

!!!WARNING!!!
nosec detected in the following files: app/ante/ante.go, common/ethereum/proof.go, contrib/rpctest/main.go, rpc/backend/account_info.go, rpc/backend/blocks.go, rpc/backend/chain_info.go, rpc/backend/tracing.go, rpc/backend/tx_info.go, rpc/backend/utils.go, rpc/namespaces/ethereum/eth/api.go, rpc/types/block.go, rpc/types/events.go, rpc/types/utils.go, x/crosschain/client/querytests/cctx.go, x/crosschain/client/querytests/chain_nonces.go, x/crosschain/client/querytests/gas_price.go, x/crosschain/client/querytests/intx_hash.go, x/crosschain/client/querytests/last_block_height.go, x/crosschain/client/querytests/out_tx_tracker.go, x/crosschain/keeper/cctx_utils.go, x/crosschain/keeper/evm_deposit.go, x/crosschain/keeper/grpc_zevm.go, x/crosschain/keeper/keeper_cross_chain_tx.go, x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go, x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go, x/crosschain/keeper/keeper_gas_price.go, x/crosschain/keeper/keeper_tss.go, x/crosschain/keeper/msg_server_whitelist_erc20.go, x/crosschain/keeper/zeta_conversion_rate.go, x/crosschain/migrations/v2/migrate.go, x/fungible/client/cli/tx_deploy_fungible_coin_zrc_4.go, x/fungible/keeper/evm.go, x/fungible/keeper/gas_coin_and_pool.go, x/fungible/keeper/msg_server_deploy_fungible_coin_zrc20.go, x/observer/abci.go, zetaclient/bitcoin_client.go, zetaclient/btc_signer.go, zetaclient/evm_client.go, zetaclient/tss_signer.go, zetaclient/utils.go, zetaclient/zetacore_observer.go

Be very careful about using #nosec in code. It can be a quick way to suppress security warnings and move forward with development, it should be employed with caution. Suppressing warnings with #nosec can hide potentially serious vulnerabilities. Only use #nosec when you're absolutely certain that the security issue is either a false positive or has been mitigated in another way.

Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203
Broad #nosec annotations should be avoided, as they can hide other vulnerabilities. The CI will block you from merging this PR until you remove #nosec annotations that do not target specific rules.

Pay extra attention to the way #nosec is being used in the files listed above.

@github-actions github-actions bot added the nosec label Sep 20, 2023
@lumtis lumtis changed the title Gosec cosmos fix: address cosmos-gosec lint issues Sep 20, 2023
@lumtis lumtis requested a review from CharlieMc0 September 21, 2023 12:55
@lumtis lumtis marked this pull request as ready for review September 21, 2023 12:55
@github-actions github-actions bot added the ci Changes to CI pipeline or github actions label Sep 25, 2023
@lumtis
Copy link
Contributor Author

lumtis commented Sep 25, 2023

@CharlieMc0 it seems the cosmos/gosec action was outdated and didn't support Go generics.
I replaced the action step with directly running the make lint-cosmos-gosec Makefile command and it seems to work.

Copy link
Member

@CharlieMc0 CharlieMc0 left a comment

Choose a reason for hiding this comment

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

LGTM

@lumtis lumtis merged commit 7991fad into develop Oct 2, 2023
@lumtis lumtis deleted the gosec-cosmos branch October 2, 2023 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking:cli ci Changes to CI pipeline or github actions nosec

Projects

None yet

Development

Successfully merging this pull request may close these issues.

resolve cosmos gosec findings - G701,G703,G704

3 participants