E money network Chain (Audit 1)

Repository: - `EMoneyChain` Commit hash: - [`b91404fefb81a9c653d1d508feeff13abcc66332`](https://github.com/EMoney-Network/EMoneyChain/tree/b91404fefb81a9c653d1d508feeff13abcc66332) Files: - `app/app.go` In the initialization of app: ```go=210 // module accounts that are allowed to receive tokens allowedReceivingModAcc = map[string]bool{ distrtypes.ModuleName: true, ``` The `allowedReceivingModAcc` is used to hardcode the module account address allowing receiving coins from the ordinary account address. Here the `DistributionAccount` is set which would break the invariant check against the balance between distribution module and the `FeePoolCommunity`. The chain node would crash due to this invariant broken scenario and if this node is under the continuously attack with full of the crisis broken transactions. It could not even to serve the node for accepting valid transactions at the same time. Furthermore, the invariant is broken when there is a scenario to export the genesis state for hard fork upgrading, it could not work due to this checker would fail forever. Although there is restricted authority enforced by the regulation module for the user to perform the corresponding transactions towards the chain, it is still highly recommended to resolve the issue if possible.

Repository: - `EmoneyChain` Commit hash: - [`8c0cf7dd5ff999c31e75774eb84c00f807f0425f`](https://github.com/EMoney-Network/EMoneyChain/tree/8c0cf7dd5ff999c31e75774eb84c00f807f0425f) Files: - `x/kyb/keeper/genesis.go` There is a single key used for storing KYBUser information, specifically `KYBAddressPrefix`. When a user successfully completes the KYB process, their status is set to `KYBStatus_STATUS_PASSED`. Conversely, if a user is removed from the KYB process, their status is updated to `KYBStatus_STATUS_NOT_PASSED`. This leads to a problem when exporting the genesis file. During the iteration of KYBUser entries, the system does not filter based on the KYB status. As a result, users with a `KYBStatus_STATUS_NOT_PASSED` status are included in the export, as demonstrated in the snippet below: **`x/kyb/keeper/genesis.go`** ```go=42 func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState { var kybUsers []string k.IterateKYBUsers(ctx, func(kybUser sdk.ValAddress) (stop bool) { kybUsers = append(kybUsers, kybUser.String()) return false }) ``` Subsequently, during the import process of the genesis file, all KYB users are assigned a status of `KYBStatus_STATUS_PASSED`, as shown in the following code snippet: **`x/kyb/keeper/genesis.go`** ```go=9 func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) { k.SetParams(ctx, genState.Params) for _, kybUser := range genState.KybUsers { k.SetAddressStatus(ctx, utils.MustValAddressFromBech32(kybUser), types.KYBStatus_STATUS_PASSED) } ``` This inconsistency with the actual status of the KYB users could lead to significant chaos within the system, potentially resulting in the retention of invalid KYB users who are incorrectly assigned a valid status of `KYBStatus_STATUS_PASSED`.

Repository: - `EmoneyChain` Commit hash: - [`b91404fefb81a9c653d1d508feeff13abcc66332`](https://github.com/EMoney-Network/EMoneyChain/tree/b91404fefb81a9c653d1d508feeff13abcc66332) Files: - `app/export.go` According to the current implementation, the function `prepForZeroHeightGenesis()` retrieves an invalid validator address from the KV store, which may be causing the export of the genesis file to fail. The function `prepForZeroHeightGenesis()` is intended to export the genesis state from the Emoney chain, which includes getting and exporting the validator information from the KV store. **`app/export.go`** ```go=146 // Iterate through validators by power descending, reset bond heights, and // update bond intra-tx counters. store := ctx.KVStore(app.keys[stakingtypes.StoreKey]) iter := sdk.KVStoreReversePrefixIterator(store, stakingtypes.ValidatorsKey) counter := int16(0) for ; iter.Valid(); iter.Next() { addr := sdk.ValAddress(iter.Key()[1:]) validator, found := app.StakingKeeper.GetValidator(ctx, addr) if !found { panic("expected validator, not found") } validator.UnbondingHeight = 0 if applyAllowedAddrs && !allowedAddrsMap[addr.String()] { validator.Jailed = true } app.StakingKeeper.SetValidator(ctx, validator) counter++ } ... ``` In line 177, the validator address is fetched via `iter.Key()[1:]`, which excludes the `ValidatorKey`, but it still includes the prefix, the length of the address. This can be demonstrated in the function `GetValidatorKey()`: **cosmos-sdk/x/staking/types/keys.go** ```go=81 // GetValidatorKey creates the key for the validator with the address // VALUE: staking/Validator func GetValidatorKey(operatorAddr sdk.ValAddress) []byte { return append(ValidatorsKey, address.MustLengthPrefix(operatorAddr)...) } ``` Therefore, the `iter.Key()[1:]` returns the bytes of the length of the address plus the address, and it cannot be correctly converted to the validator address via the function `sdk.ValAddress()`. In this case, it will cause panic because it cannot be found in the staking keeper. The export of the genesis state would fail due to this error, resulting in a failure to perform maintenance.

Repository: - `EmoneyChain` Commit hash: - [`8c0cf7dd5ff999c31e75774eb84c00f807f0425f`](https://github.com/EMoney-Network/EMoneyChain/tree/8c0cf7dd5ff999c31e75774eb84c00f807f0425f) Files: - `x/regulation/keeper/keeper.go` - `x/kyb/keeper/keeper.go` The Error message in `GetVotingById` has been silently ignored in both `KYB` and `Regulation` modules: ```go if err := k.cdc.Unmarshal(bz, voting); err != nil { return nil, nil } ```

Repository: - `EMoneyChain` Commit hash: - [`b91404fefb81a9c653d1d508feeff13abcc66332`](https://github.com/EMoney-Network/EMoneyChain/tree/b91404fefb81a9c653d1d508feeff13abcc66332) Files: - `x/lock/types/genesis.go` The aforementioned function is utilized to include a genesis vested validator in the lock genesis state, without any direct connection to regulatory authorities or regulations. Hence, the third parameter, named `regulatorAddr`, may cause confusion in terms of code readability. **`x/lock/types/genesis.go`** ```go=47 func AddGenesisValidator(cdc codec.JSONCodec, appState map[string]json.RawMessage, regulatorAddr sdk.ValAddress) error { genState := new(GenesisState) cdc.MustUnmarshalJSON(appState[ModuleName], genState) addrStr := regulatorAddr.String() ```

Repository: - `EMoneyChain` Commit hash: - [`b91404fefb81a9c653d1d508feeff13abcc66332`](https://github.com/EMoney-Network/EMoneyChain/tree/b91404fefb81a9c653d1d508feeff13abcc66332) Files: - `cmd/emoneyd/init.go` Based on the design of the `Command` struct, the `Short` property is intended to provide a brief description that appears in the `help` output, while the `Long` property is meant to present a more detailed message in the same output. However, the descriptions in scale are inverted in this instance. **`cmd/emoneyd/init.go`** ```go=66 func InitCmd(mbm module.BasicManager, defaultNodeHome string) *cobra.Command { cmd := &cobra.Command{ Use: "init [moniker]", Short: "Initialize private validator, p2p, genesis, and application configuration files", Long: `Initialize validators's and node's configuration files.`, ```

Repository: - `EmoneyChain` Commit hash: - [`b91404fefb81a9c653d1d508feeff13abcc66332`](https://github.com/EMoney-Network/EMoneyChain/tree/b91404fefb81a9c653d1d508feeff13abcc66332) Files: - `pkg/evmos-12@v12.1.6/rpc/backend/call_tx.go` Reference: - [`evmos-PR1863`](https://github.com/evmos/evmos/pull/1863) When performing the Json-RPC testing with the local node, per the API `eth_gasPrice` with following http curl command: > curl -X POST --data '{"jsonrpc":"2.0","method":"eth_gasPrice","params":[],"id":1}' -H "Content-Type: application/json" http://localhost:8545 The returned response pointed that the method crashed: ```shell {"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"method handler crashed"}} ``` Deep investigation was taken into the server side of the node with debug log printed out as ```go 9:28PM DBG eth_gasPrice client=json-rpc 9:28PM ERR RPC method eth_gasPrice crashed: runtime error: invalid memory address or nil pointer dereference goroutine 1698 [running]: github.com/ethereum/go-ethereum/rpc.(*callback).call.func1() github.com/ethereum/go-ethereum@v1.10.26/rpc/service.go:200 +0x74 panic({0x103013860?, 0x1048af370?}) runtime/panic.go:770 +0x124 github.com/evmos/evmos/v12/rpc/backend.(*Backend).GasPrice(0x140010b9908) github.com/evmos/evmos/v12@v12.1.6/rpc/backend/call_tx.go:404 +0x28 github.com/evmos/evmos/v12/rpc/namespaces/ethereum/eth.(*PublicAPI).GasPrice(0x14009552330) github.com/evmos/evmos/v12@v12.1.6/rpc/namespaces/ethereum/eth/api.go:317 +0x5c reflect.Value.call({0x140093e6280?, 0x14001540918?, 0x30?}, {0x101dcb541, 0x4}, {0x1400924fef0, 0x1, 0x101018584?}) reflect/value.go:596 +0x970 reflect.Value.Call({0x140093e6280?, 0x14001540918?, 0x0?}, {0x1400924fef0?, 0x140?, 0x14000508008?}) reflect/value.go:380 +0x94 github.com/ethereum/go-ethereum/rpc.(*callback).call(0x140093ea9c0, {0x1033d44e8, 0x140086b4d20}, {0x14005302c54, 0xc}, {0x104c5a880, 0x0, 0x101011f8c?}) github.com/ethereum/go-ethereum@v1.10.26/rpc/service.go:206 +0x2d0 github.com/ethereum/go-ethereum/rpc.(*handler).runMethod(0x14005302c60?, {0x1033d44e8?, 0x140086b4d20?}, 0x140049af960, 0x0?, {0x104c5a880?, 0x14001e77df8?, 0x100140de4?}) github.com/ethereum/go-ethereum@v1.10.26/rpc/handler.go:389 +0x40 github.com/ethereum/go-ethereum/rpc.(*handler).handleCall(0x14001158a20, 0x1400924fe90, 0x140049af960) github.com/ethereum/go-ethereum@v1.10.26/rpc/handler.go:337 +0x1d8 github.com/ethereum/go-ethereum/rpc.(*handler).handleCallMsg(0x14001158a20, 0x1400924fe90, 0x140049af960) github.com/ethereum/go-ethereum@v1.10.26/rpc/handler.go:298 +0x80 github.com/ethereum/go-ethereum/rpc.(*handler).handleMsg.func1(0x1400924fe90) github.com/ethereum/go-ethereum@v1.10.26/rpc/handler.go:139 +0x34 github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc.func1() github.com/ethereum/go-ethereum@v1.10.26/rpc/handler.go:226 +0xbc created by github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc in goroutine 1600 github.com/ethereum/go-ethereum@v1.10.26/rpc/handler.go:222 +0x84 module=geth 9:28PM INF Served eth_gasPrice conn=127.0.0.1:58262 duration=2.619792 err="method handler crashed" module=geth reqid=1 ``` Apparently, panic occurred somewhere. It could be obviously seen that there is no nil checking on the `head` before accessing it, as the code snippet of L404 shown below. **`pkg/evmos-12@v12.1.6/rpc/backend/call_tx.go`** ```go=399 func (b *Backend) GasPrice() (*hexutil.Big, error) { var ( result *big.Int err error ) if head := b.CurrentHeader(); head.BaseFee != nil { ``` There are various reasons for the header retrieval failures including the insecure grpc requesting from underlying `tendermint` or the block header not persisted in the serving node by pruning strategy. It is highly recommended to perform nil checking before accessing to the data for the invoking call.

Repository: - `EMoneyChain` Commit hash: - [`b91404fefb81a9c653d1d508feeff13abcc66332`](https://github.com/EMoney-Network/EMoneyChain/tree/b91404fefb81a9c653d1d508feeff13abcc66332) Files: - `cmd/emoneyd/genvesting.go` - `x/lock/keeper/genesis.go` The `AddGenesisVestedValidatorCmd` function in `cmd/emoneyd/genvesting.go` is tasked with adding a vested validator to the genesis state. However, it fails to incorporate a corresponding lock, specifically the `genState.Locks` for that validator. This oversight results in an inconsistency with the verification in `x/lock/keeper/genesis.go`, which anticipates that the number of locks and vested validators in the genesis state will be equal. **`x/lock/keeper/genesis.go`** ```go=12 if len(genState.Locks) != len(genState.VestedValidators) { panic("Lock and Validators length mismatch") } ``` --- Updated with commit [`8c0cf7dd5ff999c31e75774eb84c00f807f0425f`](https://github.com/EMoney-Network/EMoneyChain/tree/8c0cf7dd5ff999c31e75774eb84c00f807f0425f), the file is renamed as `cmd/emoneyd/genkybusers.go`.

Repository: - `EmoneyChain` Commit hash: - [`b91404fefb81a9c653d1d508feeff13abcc66332`](https://github.com/EMoney-Network/EMoneyChain/tree/b91404fefb81a9c653d1d508feeff13abcc66332) Files: - `pkg/evmos-12@v12.1.6/x/revenue/v1/keeper/evm_hooks.go` In the `PostTxProcessing`, only the `params.EnableRevenue` was validated before continuing the following process. **`pkg/evmos-12@v12.1.6/x/revenue/v1/keeper/evm_hooks.go`** ```go=53 func (k Keeper) PostTxProcessing( ctx sdk.Context, msg core.Message, receipt *ethtypes.Receipt, ) error { // check if the fees are globally enabled params := k.GetParams(ctx) if !params.EnableRevenue { return nil } contract := msg.To() if contract == nil { return nil } // if the contract is not registered to receive fees, do nothing revenue, found := k.GetRevenue(ctx, *contract) if !found { return nil } withdrawer := revenue.GetWithdrawerAddr() if len(withdrawer) == 0 { withdrawer = revenue.GetDeployerAddr() } txFee := sdk.NewIntFromUint64(receipt.GasUsed).Mul(sdk.NewIntFromBigInt(msg.GasPrice())) developerFee := (params.DeveloperShares).MulInt(txFee).TruncateInt() evmDenom := k.evmKeeper.GetParams(ctx).EvmDenom fees := sdk.Coins{{Denom: evmDenom, Amount: developerFee}} // distribute the fees to the contract deployer / withdraw address err := k.bankKeeper.SendCoinsFromModuleToAccount( ctx, k.feeCollectorName, withdrawer, fees, ) if err != nil { return errorsmod.Wrapf( err, "fee collector account failed to distribute developer fees (%s) to withdraw address %s. contract %s", fees, withdrawer, contract, ) ``` However, the scenario of that the `EnableRevenue` was set with the `DeveloperShares` to be zero should also bypass in this case, otherwise the error would be popped in the `SendCoinsFromModuleToAccount` with zero `fees`. It's recommended to take the validation on the `DeveloperShares` against zero as well. Reference: - [`evmos-pr1659`](https://github.com/evmos/evmos/pull/1659)

Repository: - `EMoneyChain` Commit hash: - [`b91404fefb81a9c653d1d508feeff13abcc66332`](https://github.com/EMoney-Network/EMoneyChain/tree/b91404fefb81a9c653d1d508feeff13abcc66332) Files: - `x/regulation/handler.go` - `x/regulation/keeper/msg_server.go` The following functions from msg_server.go are not being called in `handler.go`: - ProposeBlock - ProposeAdd - Vote - ProcessVoting This means that the message types corresponding to these functions (MsgProposeBlock, MsgProposeAdd, MsgVote, MsgProcessVoting) will not be handled by the regulation module.