EthereumCommonwealth / Auditing

Ethereum Commonwealth Security Department conducted over 400 security audits since 2018. Not even a single contract that we audited was hacked. You can access our audit reports in the ISSUES of this repo. We are accepting new audit requests.

https://audits.callisto.network/

GNU General Public License v3.0

132 stars 34 forks source link

ETC dice #1

Closed craz3049 closed 6 years ago

craz3049 commented 6 years ago

Audit request

basically we seek to develop traditional casino games by implementing decentralization using the blockchain of ethereum classic, in the future we will seek to improve the use and interaction of contracts

Source code

https://github.com/craz3049/etc-dice-game

Disclosure policy

rafa940325@gmail.com

Platform

was deployed on ethereum classic

yuriy77k commented 6 years ago

Auditing time 1 day.

Dexaran commented 6 years ago

@yuriy77k assigned.

MrCrambo commented 6 years ago

Auditing time: 1 day

Dexaran commented 6 years ago

@MrCrambo assigned

dieselc commented 6 years ago

Auditing time: 2 days

Dexaran commented 6 years ago

@dieselc assigned.

rhyzome commented 6 years ago

Auditing time: 2 days

Dexaran commented 6 years ago

@rhyzome assigned.

RideSolo commented 6 years ago

Auditing time: 1 day

alexo18 commented 6 years ago

Auditing time 1 day.

gorbunovperm commented 6 years ago

Estimated auditing time is 1 day.

Dexaran commented 6 years ago

@RideSolo @alexo18 @gorbunovperm assigned.

gorbunovperm commented 6 years ago

@Dexaran My report is complete.

Dexaran commented 6 years ago

@gorbunovperm @yuriy77k @MrCrambo @dieselc @RideSolo @rhyzome @alexo18

I will ask you to send an email with the following information to dexaran@callisto.network:

your github profile
your payment address
position that you applied for
your audit report gist URL
discord server nickname if you are at our discord (join discord here)

I will revel reports in 3 days or as soon as I receive this information from each participant.

Dexaran commented 6 years ago

Revealing audit reports:

https://gist.github.com/alexo18/e804cf89a1eb39dc60928e24a771d99c https://gist.github.com/yuriy77k/e52201c4552b2b76cc70379c730d1e37 https://gist.github.com/RideSolo/8faa6fc13f5f659d78970829aaf0116f https://gist.github.com/gorbunovperm/5460648066ef74676ae34a077a7ab6af https://gist.github.com/dieselc/708393afeab17227d1a8584ab14b54c3 https://gist.github.com/MrCrambo/3f9b202aedc23a94bb91a0037488d976

Dexaran commented 6 years ago

Conclusion: bug fixing is necessary

The ETC dice game contract has critical issues. Therefore, deployment is prohibited and the contract should not be used. It is necessary to fix the contract.

Critical issues:

Usage of block.timestamp. A miner can manipulate game results.
Wrong if condition at FlipCoin() function. #L24

Medium severity issues:

Uninitialized owner variable

Dexaran commented 6 years ago

@alexo18

Notes regarding the https://gist.github.com/alexo18/e804cf89a1eb39dc60928e24a771d99c report.

You haven't reported the critical issue: "block.timestamp manipulation".
Missing function visibility specifier does not pose any threat. As the result, the minor observation/not a security issue severity should be assigned.

Conclusion

The reported issues can not directly hurt the dice-game smart-contract. The dice-game smart-contract can satisfy the main goal and could be used for dice-game contract after completion aforementioned bugs list.

Contract can not satisfy the main goal since the results could be manipulated.

Hire

Your application for junior security auditor is approved. However, I strongly recommend that you review other audit reports and study the programming of contracts more thoroughly.

Dexaran commented 6 years ago

@yuriy77k

Notes regarding the https://gist.github.com/yuriy77k/e52201c4552b2b76cc70379c730d1e37 report.

Your audit report is fine.

One note about the conclusion:

One critical vulnerability was detected.
It is highly recommended to complete a bug bounty before use.

There is no need to run a bugbounty for a contract which has a critical vulnerability. Bugfixing is necessary.

Hire

Your application for medium security auditor is approved.

Dexaran commented 6 years ago

@RideSolo

Notes regarding the https://gist.github.com/RideSolo/8faa6fc13f5f659d78970829aaf0116f report.

Result precomputation make sense. However, this is an issue of block.timestamp usage. A block timestamp can not serve as a source of entropy for a true random number generator.
Timestamp manipulation can directly harm the results of the game. It is better to assign it high severity.

Hire

Your application for medium security auditor position is approved.

Dexaran commented 6 years ago

@gorbunovperm

Notes regarding the https://gist.github.com/gorbunovperm/5460648066ef74676ae34a077a7ab6af report.

It is impossible to withdraw funds from the contract to the owner's account (link) - this issue can not be directly exploited AND this issue can not harm anyone apart from the owner of the contract. It is better to assign it medium severity.

Hire

Your application for junior security auditor is approved.

Dexaran commented 6 years ago

@dieselc

Notes regarding the https://gist.github.com/dieselc/708393afeab17227d1a8584ab14b54c3 report.

Transaction Ordering Dependence - definitely, this issue make sense, but this is not directly related to this smart-contract. This is more the issue of protocol-level.
#L24 condition can directly harm a player. Therefore, it is better to assign it medium or high severity.

Hire

Your application is approved. You will be assigned the role of a medium security auditor.

Dexaran commented 6 years ago

@MrCrambo

Notes regarding the https://gist.github.com/MrCrambo/3f9b202aedc23a94bb91a0037488d976 report.

It is better to describe issues separately. For example Old solidity version and not actual suicide function should be divided into "Old solidity version" (minor) and "No actual suicide function" (low) issues.
This recommendation is apparently copied from the previous item. selfdestruct opcode implementation can not solve the CoinFlip function.

Hire

Your application is approved. You will be assigned the role of a medium security auditor.

Dexaran commented 6 years ago

General notes regarding the audit reports

It is up to security auditor to assign a severity of an issue.
It is up to auditing manager to summarize and assign a final severity to each reported issue.
If the severity of an issue described by the auditor does not match the severity that the auditing manager finally assigned, then this should not be considered as auditors mistake. (The most important here is the fact that the issue was reported)
If an auditor has not reported a medium/high severity issue, then it must be considered as auditors mistake.
Compiler default warnings, readability issues and missing function visibility specifiers should be assigned the minor observation/ not a security issue severity.