Certik review 09/11
ngdlong91 opened this issue · 1 comments
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 |
P1 Completed