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
131 stars 34 forks source link

POCgames #69

Closed yuriy77k closed 6 years ago

yuriy77k commented 6 years ago

Audit request

I am a developer of POC game team. My name is Alejandroj Bettini. This is POC team's website. http://pocgames.co/ This is a website for this game. http://efifty.pocgames.co/ This smart contract is betting game. It is going to be used on Ethereum/Ethereum classic network. This contract is doing like this.1) Win rate is 50%.2) if win 1.9x goes to winner, if lose, get nothing3) 2% is used for developer fee4) 1% goes to jackpot (jackpot win rate is 0.1%) - jackpot is stacked5) 2% goes to whale - it is all given to POC and ePOC token(our team's ERC20 token) owners. Our token is diviend token, you can see our token here. http://etherhub.io/addr/0x08f7039d36f99eedc3d8b02cbd19f854f7dddc4d#tab_addr_36

How game play work: First, bot create a ticket with ticketID and ticketReveal(random string). When buy, users only know ticketID. ticketReveal is not shared till the result is published. ticketID is a signature that is signed by secretSigner. secretSigner is an ETH/ETC account, bot contains its private key and contract contains it's public key. There's wager function which buy ticket using the ticketID created by bot. Bot wait for Wager Event to happen and if detect it, automatically call play function of this contract. play function determine if player is winner or loser and if win provide 1.9 x betAmount, if lose provide 0. Also if user win jackpot, he/she will get whole amount of jackpot. If contract has low balance users can't bet on the contract, that case, administrators feed the contract for its health.Purpose of this audit is to have 100% audited game contract.Thanks for your effort.POC team

Source code

https://gist.github.com/yuriy77k/909d9541f30628d3a0e6c39af36a4cdf

the same as http://etherhub.io/addr/0x489633589a2e9285197cabea82ba6a8af99acdd7#tab_addr_3

Disclosure policy

alejanbettini@outlook.com

Platform

ETC/ETH

Complexity

Low

yuriy77k commented 6 years ago

You could follow the audit queue there: https://github.com/EthereumCommonwealth/Auditing/blob/master/queue.md

alejbettini121 commented 6 years ago

Thank you @yuriy77k

alejbettini121 commented 6 years ago

I will get message to my email (alejanbettini@outlook.com) when audit finished? @yuriy77k

yuriy77k commented 6 years ago

I will get message to my email (alejanbettini@outlook.com) when audit finished? @yuriy77k

Yes

pro100skm commented 6 years ago

Auditing time: 2 days

yuriy77k commented 6 years ago

@pro100skm assigned

danbogd commented 6 years ago

Аuditing time: ~3 days.

yuriy77k commented 6 years ago

@danbogd assigned

RideSolo commented 6 years ago

Auditing time: 2~ days

yuriy77k commented 6 years ago

@RideSolo assigned

danbogd commented 6 years ago

My report is finished.

yuriy77k commented 6 years ago

The contract contains high severity issues. The report was sent to the developer.

yuriy77k commented 6 years ago

@danbogd Notes about the report: https://gist.github.com/danbogd/7ccbc38f82605f3fed4fa26bd875ec30

3.2. No protection from overflows/underflows.. No overflow in specified places.

yuriy77k commented 5 years ago

1. Summary

POCgames security audit report performed by Callisto Security Audit Department

2. In scope

https://gist.github.com/yuriy77k/909d9541f30628d3a0e6c39af36a4cdf

the same as http://etherhub.io/addr/0x489633589a2e9285197cabea82ba6a8af99acdd7#tab_addr_3

3. Findings

3 issues were reported including:

3.1. Result Pre-Calculation

Severity: High

Description

wager function parameters ticketID and ticketLastBlock form a message that is signed externally using web3.eth.sign the signature is made of 3 variables v, r, s that are used as input parameters also to recover the secretSigner address and confirm that the ticketID was provided by the secretSigner address private key owner.

However, secretSigner state variable is assigned the value of ecrecover(signatureHash, v, r, s) output, then a requirement is set with:

 require (secretSigner == ecrecover(signatureHash, v, r, s), "web3 vrs signature is not valid.");

The condition will always be true since secretSigner was assigned with the value of ecrecover(signatureHash, v, r, s), any user can call wager and put a bet.

If an attacker calculate ticketID using uint(keccak256(abi.encodePacked(ticketReveal))) with a value of ticketReveal of his choice, the attacker can call wager place a bet using the calculated ticketID. (using the setup ticketReveal will allow him to call play function and retrieve his ticketID)

play function result can be pre-calculated before calling it using the setup ticketReveal value, if the result allow the attacker to win, the attacker will call play otherwise he will wait 250 blocks and call refund.

The bot set up by the project team will not be able to call play to run the bet of the attacker since the ticketReveal will be unknown to it.

Code snippet

https://gist.github.com/RideSolo/d4b8cc709953428ad5b2bda8767f83b4#file-fiftyflip-sol-L136

https://gist.github.com/RideSolo/d4b8cc709953428ad5b2bda8767f83b4#file-fiftyflip-sol-L137

https://gist.github.com/RideSolo/d4b8cc709953428ad5b2bda8767f83b4#file-fiftyflip-sol-L160

https://gist.github.com/RideSolo/d4b8cc709953428ad5b2bda8767f83b4#file-fiftyflip-sol-L156#L201

3.2. Donation Withdrawal

Severity: medium

Description

If a donator withdraw an amount higher than address(this).balance - lockedInBets - jackpotSize - devFeeSize using withdrawDonation, checkContractHealth modifier will not allow multiple functions to execute, the contract will freeze most actions until a new donation is done or ether is sent to contract through the fallback function.

donator should not be allowed to withdraw an amount higher than address(this).balance - lockedInBets - jackpotSize - devFeeSize.

The consequences can vary widely for both the project team and users.

Code snippet

https://gist.github.com/RideSolo/d4b8cc709953428ad5b2bda8767f83b4#file-fiftyflip-sol-L211#L219

3.3. No checking for zero address.

Severity: low

Description

Functions member of FiftyFlip contract do not require the to address to be non null before transfer. Accidental token loss to address 0x0 can be applicable.

Code snippet

https://gist.github.com/yuriy77k/909d9541f30628d3a0e6c39af36a4cdf#file-fiftyflip-sol-L74

https://gist.github.com/yuriy77k/909d9541f30628d3a0e6c39af36a4cdf#file-fiftyflip-sol-L107

https://gist.github.com/yuriy77k/909d9541f30628d3a0e6c39af36a4cdf#file-fiftyflip-sol-L114

4. Conclusion

The contract is not safe, a direct exploit has been highlighted.

5. Revealing audit reports

https://gist.github.com/yuriy77k/f2379724799ab771db12b3f0a68f9caa

https://gist.github.com/yuriy77k/9ddab2726b99c22db3716fbfab1903f6

https://gist.github.com/yuriy77k/0ba71a138f64df16fa7a0eaa3980c31f