Platform: Code4rena
Start Date: 28/06/2022
Pot Size: $25,000 USDC
Total HM: 14
Participants: 50
Period: 4 days
Judge: GalloDaSballo
Total Solo HM: 7
Id: 141
League: ETH
Rank: 33/50
Findings: 1
Award: $44.83
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: zzzitron
Also found by: 0v3rf10w, 0x1f8b, 0x29A, AlleyCat, Bnke0x0, Chom, Funen, JC, Lambda, Limbooo, Meera, Picodes, Sm4rty, TerrierLover, TomJ, __141345__, asutorufos, aysha, c3phas, cccz, defsec, fatherOfBlocks, grGred, hake, ignacio, ladboy233, mrpathfindr, oyc_109, rfa, sach1r0, samruna, slywaters, ynnad
44.8261 USDC - $44.83
The ioutil
package is deprecated. os.ReadFile
should be used instead of ioutil.ReadFile
Reference:
The following lines of code are affected:
x/unigov/client/cli/utils.go:17: contents, err := ioutil.ReadFile(filepath.Clean(metadataFile)) x/unigov/client/cli/utils.go:38: contents, err := ioutil.ReadFile(filepath.Clean(metadataFile)) x/vesting/client/cli/tx.go:66: contents, err := ioutil.ReadFile(filepath.Clean(path)) x/erc20/client/cli/utils.go:15: contents, err := ioutil.ReadFile(filepath.Clean(metadataFile))
Comments in production code should not contain developer discussion or notes about known bugs or problems. These issues should be tracked elsewhere and resolved before being deployed.
Comments of this kind can also indicate potential avenues of attack for an adversary.
The following lines are affected:
app/app.go:813: // TODO: Record the count along with the code and or reason so as to display app/ante/comission.go:14:// TODO: remove once Cosmos SDK is upgraded to v0.46 app/ante/vesting.go:78:// TODO: remove once Cosmos SDK is upgraded to v0.46 app/tps_counter.go:113: // TODO: Perhaps log this? testutil/network/network.go:71: LegacyAmino *codec.LegacyAmino // TODO: Remove! x/vesting/types/schedule.go:167:// TODO: rename and add comprehensive comments, this is currently not maintainable cmd/cantod/root.go:94: // TODO: define our own token cmd/cantod/root.go:101: // TODO: double-check cmd/cantod/main.go:35: // TODO fix cmd/config/observability.go:28: // // TODO: Derive the Prometheus observability exporter from the Evmos config.
Packages used by the project are known to contain security vulnerabilities. It is recommended to update these dependencies in order to avoid issues related to these vulnerabilities.
In addition, it is advised to put in place an automated process to flag vulnerable components and fix them during the build process.
The following vulnerable packages are installed:
For more information consult the following resources:
Tendermint GitHub Issue Go Ethereum security advisory
Comments in production code should not contain developer discussion or notes about known bugs or problems. These issues should be tracked elsewhere and resolved before being deployed.
Comments of this kind can also indicate potential avenues of attack for an adversary.
The following lines are affected:
testutil/network/network.go: LegacyAmino *codec.LegacyAmino // TODO: Remove! x/vesting/types/schedule.go:// TODO: rename and add comprehensive comments, this is currently not maintainable app/ante/comission.go:// TODO: remove once Cosmos SDK is upgraded to v0.46 app/ante/vesting.go:// TODO: remove once Cosmos SDK is upgraded to v0.46 app/app.go: // TODO: Record the count along with the code and or reason so as to display app/tps_counter.go: // TODO: Perhaps log this? cmd/config/observability.go: // // TODO: Derive the Prometheus observability exporter from the Evmos config. cmd/cantod/root.go: // TODO: define our own token cmd/cantod/root.go: // TODO: double-check cmd/cantod/main.go: // TODO fix
Avoid using panic
in production code. Calls to panic
can reveal sensitive information about the system via the output of stack traces. When errors are not handled in a recoverable way it is possible for the software to reach undefined behaviour or even crash.
Instead of using panic
, incorporate custom errors according to the best practices of the CosmosSDK.
More information about custom errors can be found at the following resource:
The use of panic was identified at the following lines in the codebase:
x/unigov/genesis.go:17: panic("the UniGov module account has not been set") x/unigov/client/cli/tx.go:118: panic(err) x/unigov/client/cli/tx.go:121: panic(err) x/unigov/client/cli/tx.go:124: panic(err) x/unigov/client/cli/tx.go:204: panic(err) x/unigov/client/cli/tx.go:207: panic(err) x/unigov/client/cli/tx.go:210: panic(err) x/unigov/simulation/simap.go:12: panic(err) x/recovery/module.go:75: panic(err) x/inflation/module.go:79: panic(err) x/recovery/keeper/keeper.go:57: panic("ICS4 wrapper already set") x/epochs/module.go:79: panic(err) x/vesting/module.go:66: panic(err) x/epochs/keeper/keeper.go:31: panic("cannot set epochs hooks twice") x/vesting/keeper/msg_server.go:228: panic("bad start time calculation") x/inflation/genesis.go:19: panic("the inflation module account has not been set") x/inflation/keeper/epoch_mint_provisions.go:21: panic(fmt.Errorf("unable to unmarshal epochMintProvision value: %w", err)) x/inflation/keeper/epoch_mint_provisions.go:31: panic(fmt.Errorf("unable to marshal amount value: %w", err)) x/inflation/keeper/keeper.go:39: panic("the mint module account has not been set") x/incentives/module.go:75: panic(err) x/inflation/keeper/hooks.go:42: panic("the epochMintProvision was not found") x/inflation/keeper/hooks.go:47: panic(err) x/incentives/genesis.go:22: panic("the incentives module account has not been set") x/incentives/client/cli/tx.go:91: panic(err) x/incentives/client/cli/tx.go:94: panic(err) x/incentives/client/cli/tx.go:97: panic(err) x/incentives/client/cli/tx.go:163: panic(err) x/incentives/client/cli/tx.go:166: panic(err) x/incentives/client/cli/tx.go:169: panic(err) x/incentives/keeper/allocation_meters.go:51: panic(fmt.Errorf("unable to unmarshal amount value %v", err)) x/incentives/keeper/allocation_meters.go:63: panic(fmt.Errorf("unable to marshal amount value %v", err)) x/incentives/keeper/epoch_hooks.go:27: panic(err) x/erc20/genesis.go:23: panic("the erc20 module account has not been set") x/fees/module.go:80: panic(err) x/erc20/module.go:74: panic(err) x/erc20/module.go:139: panic(fmt.Errorf("failed to migrate %s to v2: %w", types.ModuleName, err)) x/erc20/client/cli/tx.go:222: panic(err) x/erc20/client/cli/tx.go:225: panic(err) x/erc20/client/cli/tx.go:228: panic(err) x/erc20/client/cli/tx.go:288: panic(err) x/erc20/client/cli/tx.go:291: panic(err) x/erc20/client/cli/tx.go:294: panic(err) x/erc20/client/cli/tx.go:354: panic(err) x/erc20/client/cli/tx.go:357: panic(err) x/erc20/client/cli/tx.go:360: panic(err) app/tps_counter.go:77: panic(err) app/tps_counter.go:83: panic(err) contracts/erc20burnable.go:21: panic(err) app/forks.go:30: panic(err) app/forks.go:50: panic(err) contracts/erc20DirectBalanceManipulation.go:31: panic(err) contracts/erc20DirectBalanceManipulation.go:35: panic("load contract failed") contracts/erc20.go:29: panic(err) contracts/erc20.go:33: panic("load contract failed") contracts/erc20maliciousdelayed.go:31: panic(err) contracts/erc20maliciousdelayed.go:35: panic("load contract failed") app/test_helpers.go:70: panic(err) app/test_helpers.go:77: panic(err) contracts/ProposalStore.go:21: panic(err) ibc/testing/chain.go:55: panic(err) app/app.go:148: panic(err) app/app.go:769: panic(err) app/app.go:829: panic(err) app/app.go:981: panic(err) app/app.go:1050: panic(fmt.Errorf("failed to read upgrade info from disk: %w", err)) cmd/config/observability.go:33: // panic(err) cmd/config/config.go:51: panic(err) cmd/config/config.go:55: panic(err) cmd/cantod/main.go:37: // panic(err) cmd/cantod/root.go:134: panic(err) cmd/cantod/root.go:204: panic(fmt.Errorf("unknown app config type %T", customAppConfig)) cmd/cantod/root.go:232: panic(err) cmd/cantod/root.go:238: panic(err) cmd/cantod/root.go:242: panic(err)
#0 - GalloDaSballo
2022-08-14T19:46:04Z
Valid Low
NC
In lack of POC I cannot count this one as valid, please show how the vulnerability is exposed in the code in scope next time
Valid Refactoring
1 L 1R 1NC