Certik review 09/11

[ngdlong91]

Review report

PRE-KardiaChain-09_11_2020.html.zip

Created issues:

[CodeReview] Moves error into dedicated files #76
[CodeReview] Removes code panics #75
[CodeReview] Removes math/rand #74

Tasks

• Use of weak random number generator

The code base is using math/rand, the random generation is weak and its not suited for sensitive implementations.
The implementation is part of the tool/ folder and it is part of the testing suite.
This exhibit is more of a warning rather than a critical finding. It serves as a reminder to avoid math.rand

PR #74

• Code Panic

The code panics in various places without making sure that running processes are terminated properly.
This exhibit is just a example of the identified issue. The issue exists in various places within the codebase and its amplified by the fact that we are going to be running two main-net chains.

Reply: Code panic at the moments panic while starting node/network, wrong state data or consensus errors
For this milestones, we are sure all the panic code sections well-handed in its case.

• Unsafe Pointer Access

Usafe memory access as the code base makes assumption that tx will be never nil.
This exhibit is a example, and while the code base follows a good handling of those issues for the most part of the code, some parts need refactoring for a more secure outcome under any case.
dbd705c

• Function Visibility

The function is public and although it does not need to be.
ac32f8a

• Redundant constant declaration

We recommend not declaring return variables on the functions signature.

• Redundant Code

Redundant variable declaration.
KAR-05: Redundant const declaration > 45f993c
KAR-06-13: 4ed5e4b

• Passing lock by value

Function is passing lock by value as BlockChain contains a mutex.
fc477ee

• Defer usage

It is recommended to use defer in cases that are non access specific.
KAR-06: Return variable declaration >61c727f

Issues by severity

Critical

ID	Title	Example	Status
KAR-01	Use of weak random number generator	https://github.com/kardiachain/go-kardiamain/blob/6b2bec4fa98a80650280aaeb2d913f6c5fef8dad/tool/tx_generator.go#L284

Major

ID	Title	Example	Status
KAR-02	Code Panic	https://github.com/kardiachain/go-kardiamain/blob/ccf7672814d10ada7158803282dc873bb2b35523/kai/state/statedb.go#L576

Minor

ID	Title	Example	Status
KAR-03	Unsafe Pointer Access	https://github.com/kardiachain/go-kardiamain/blob/6b2bec4fa98a80650280aaeb2d913f6c5fef8dad/mainchain/tx_pool/tx_pool.go#L633

Informational

ID	Title	Example	Status
KAR-04	Function Visibility	https://github.com/kardiachain/go-kardiamain/blob/6b2bec4fa98a80650280aaeb2d913f6c5fef8dad/tool/tx_generator.go#L284
KAR-05	Redundant constant declaration	https://github.com/kardiachain/go-kardiamain/blob/6b2bec4fa98a80650280aaeb2d913f6c5fef8dad/tool/tx_generator.go#L284
KAR-06	Return variable declaration	https://github.com/kardiachain/go-kardiamain/blob/6b2bec4fa98a80650280aaeb2d913f6c5fef8dad/consensus/types/height_vote_set.go#L120
KAR-07	Redundant Code	https://github.com/kardiachain/go-kardiamain/blob/6b2bec4fa98a80650280aaeb2d913f6c5fef8dad/lib/events/events.go#L56
KAR-08	Passing lock by value	https://github.com/kardiachain/go-kardiamain/blob/6b2bec4fa98a80650280aaeb2d913f6c5fef8dad/lib/events/events.go#L56
KAR-09	Defer Usage

[lewtran]

P1 Completed