- 1 - Table of Contents
- 2 - Introduction
- 2.1 - Authenticity
- 2.2 - Audit Goals and Focus
- 2.2.1 - Sound Architecture
- 2.2.2 - Smart Contract Best Practices
- 2.2.3 - Code Correctness
- 2.2.4 - Code Quality
- 2.2.5 - Security
- 2.3 - About Etheroll
- 2.4 - Terminology
- 3 - Overview
- 3.1 - Source Code
- 3.2 - Contracts
- 4 - General Findings
- 4.1 - General Thoughts
- 4.2 - Minor Issues
- 4.3 - Medium Issues
- 4.4 - Major Issues
- 4.5 - Critical Issues
- 4.6 - Test Coverage Analysis
- 5 - Detailed Findings
From March 24th through March 31st of 2017, Piper Merriam conducted an audit of the Etheroll smart contracts. The findings of the audit are presented in this document.
This audit was performed under a contracted hourly rate with no other compensation.
Disclaimer that Piper Merriam currently holds 10,100 etheroll tokens purchased through the crowdsale.
This document should have an attached cryptographic signature to ensure it has not been tampered with. The signature can be verified using the public key from Piper Merriam's the keybase.io.
This audit includes both objective findings from the contract code as well as subjective assessments of the overall architecture and design choices. Given the subjective nature of certain findings it will be up to the Etheroll development team to determine the appropriate response to each issue.
This audit will evaluate whether the codebase follows the current established best practices for smart contract development.
This audit will evaluate whether the code does what it is intended to do.
This audit will evaluate whether the code has been written in a way that ensures readability and maintainability.
This audit will look for any exploitable security vulnerabilities, or other potential threats to the integrity of the Etheroll system. Primary focuses are:
- Ensure investor funds in the bankroll are secure.
- Ensure that user funds are secure.
- Ensure that the game rules are accurately represented in the code.
The primary Etheroll website can be found at etheroll.com. That website has this to say about Etheroll.
Etheroll is an Ethereum smart contract for placing bets on our provably-fair dice game using Ether with no deposits or sign-ups. Each dice roll is provably random and cryptographically secure thanks to the nature of the Ethereum blockchain.
This audit uses the following terminology.
Measurement of the testing code coverage. This measurement was done via inspection of the code.
No tests.
The tests do not cover some set of non-trivial functionality.
The tests cover all major functionality.
The tests cover all code paths.
Measurement of magnitude of an issue.
Minor issues are generally subjective in nature, or potentially deal with topics like "best practices" or "readability". Minor issues in general will not indicate an actual problem or bug in code.
The maintainers should use their own judgement as to whether addressing these issues improves the codebase.
Medium issues are generally objective in nature. Most medium level issues will not represent an actively exploitable bugs or security problem, but rather an issue that is likely to lead to a future error or security issue.
In most cases a medium issue should be addressed unless there is a clear reason not to.
Major issues will be things like bugs or security vulnerabilities. These issues may not be directly exploitable such as requiring a specific condition to arise in order to be exploited.
Left unaddressed these issues are highly likely to cause problems with the operation of the contract or lead to a situation which allows the system to be exploited in some way.
Critical issues are directly exploitable bugs or security vulnerabilities.
Left unaddressed these issues are highly likely or guaranteed to cause major problems or potentially a full failure in the operations of the contract.
The smart contract source code was made available in a private repository on github.
The code being evaluated was found under the commit hash
5a1ab3b3c966f48c10f37c0047966aac191eb6dd
This audit covers the following single Solidity source file.
./contracts/Etheroll.sol
The sha256 of that contract source is as follows.
$ shasum -a 256 ./contracts/Etheroll.sol
6792724057484bda1ff7fbf86f98ab8654a7f43aacd157712e0d8ea5d1ff2a48 ./contracts/Etheroll.solThes source file ./contracts/Etheroll.sol contains a number of abstract base
contracts which are combined into the single Etheroll contract.
This section contains general findings which are not necessarily related to a specific part of the contract code.
The changes to the contract code between the previous audit and this one are largely superficial.
- Changed
betIdtype frominttobytes32 - Bet validity now checks against profit rather than bet amount.
- Rename
maxBetDivisortomaxProfitDivisor - Rename
maxBettomaxProfit - Rename
maxBetAsPercentOfHousetomaxProfitAsPercentOfHouse - Add priviledged function for updating oraclize cost.
- Track total Wei that has been won.
- Addition of
LogOwnerTransferevent. - Addition of
LogRefundevent. - Modify how random roll result is computed.
- Addition of 1 Wei consolation prize.
Examination of these changes did not yield any new exploitable issues.
When a player loses their bet, they are sent 1 wei as a sort of consolation
prize. At the current price of ether, this is $0.00000000000000005. The
transaction cost for sending this amount is multiple orders of magnitude than
the amount itself.
In cases where the sending fails, this amount is set aside for the user to withdraw. Withdrawal of this amount will cost roughly 14 orders of magnitude more than the amount itself meaning these rewards are likely to never be withdrawn.
The Oraclize service is capable of controlling the result of dice rolls. The process to do this is as follows.
- Make random.org API call
- Generate TLS proof
- Generate IPFS URI for proof
- Precompute
sha3(.....) % 100 + 1. - If the result is the desired roll, submit result to the
__callbackfunction. - Otherwise return to step 1.
While the Etheroll contract cannot prevent this action, it does take steps to ensure it is dectectable. Each API result returned by Random.org contains a serial number which increases by 1 for each API request.
The Etheroll contract logs this value making this type of exploit detectable.
No change recommended for this issue.
None
None
None
The test coverage for this codebase appears to not have been changed since the previous audit.
The tests cover a small number of utility functions but do not appear to cover any of the core functionality.
The following is an exhaustive list of all issues found during in this audit.
The following formula is used to compute a player's winnings.
((((msg.value * (100-(safeSub(rollUnder,1)))) / (safeSub(rollUnder,1))+msg.value))*houseEdge/houseEdgeDivisor)-msg.value;The formula is constrained as follows.
- The
rollUndervalue is constrained between 2 and 99 (inclusive). - The
houseEdgeis set to990 - The
houseEdgeDivisoris set to1000 - The
msg.valueis required to be100000000000000000(0.1 ether) or greater.
This formula will overflow for very small msg.value values. This is likely
not a concern since the minimum bet is likely to always be sufficiently high.
In addition, in the event that it does underflow, it will exceed the
maxProfit value checks.
Recommend just being aware that this is possible in case future changes to this formula alter the underflow conditions.
The places that log the LogResult event use integer values for the status
field.
LogResult(...., 2, ...); There is a comment accompanying the declaration of the LogResult event which
denotes what each of these integer values represent.
/* Status: 0=lose, 1=win, 2=win + failed send, 3=refund, 4=refund + failed send*/
event LogResult(bytes32 indexed BetID, address indexed PlayerAddress, uint indexed PlayerNumber, uint DiceResult, uint Value, int Status, bytes Proof); While it is good that there is a reference, it makes the code more difficult to read.
Recommend either using an Enum to represent these status values or using
constant variables for each.
enum Status {
Lose,
Win,
WinWithSendFail,
Refund,
RefundWithSendFail
}
// or
uint constant LOSE = 0;
uint constant WIN = 0;
uint constant WIN_WITH_SEND_FAIL = 0;
uint constant REFUND = 0;
uint constant REFUND_WITH_SEND_FAIL = 0;
The formula for calculating winnings is duplicated in two locations.
- Line 302
- Line 450
Any changes to the formula must be made in two places. This type of duplication can lead to errors, such as the case where only one of these formulas is modified, resulting in inconsistent business logic.
Recommend creating a function which does this computation.
None
None