search

PwayGames / PWay.Contracts

1 stars 5 forks source link

Hunt for bugs #1

Open adamskrodzki opened 5 years ago

adamskrodzki commented 5 years ago

In PWay we value security a lot. therefore we announce bug bounty of 5 ETH.

In order to motivate users for search, we hide 3 bugs in a code, they have combine price of 1 ETH all other bugs are priced depends on severity , but maximal is 1 ETH .

Bellow we includes results of following invocation web3.sha3("bug description")

0xcc6fa58ea9e1adfead6d81909c9ecbd7d9014d0bd8fa37bad63c43ba8ecb2e41 - 0,5 ETH Price

0xa3633a40fbdc1cddd982ed2483613e467229c549e477c20fe5961592ebe7dc0b - 0,3 ETH Price 0x9247b4c67bf0a167d79a1d94162b84fa90c90c8e8c3a26d4eff8fb38fb9f5a87 - 0,2 ETH Price already completed by @jakub-wojciechowski

All hashes relates to following commit https://github.com/PwayGames/PWay.Contracts/commit/82b3ba7a0d197ac1ec5489b4699352f3bcf08501

If You like to test on public testnet (Rinkeby) here are some of the addresses //NameRegistry //0x500f2783a2b8b821923f52c654e11af75d18000b //PwayToken //0x5cefb431A0C17E10a134CBfa34c5Cbfa5C1BEb22 //PwayAuthority //0x04853f0150318a949ECAD2627BFE0e4e7C8e500E //PwayCompany //0xe9313B25eEB25ea0F1E4698FCc58D366ae280eb9 //PwayCrowdsale //0xD9A1134cdf6b3B7E3580ED663B8e7F1Ddc8d58e2

Definition of done Writing a scenario (in form of a test) where one of things happens:

Critical Bugs- 1 ETH

  1. Attacker can become an owner of PWay/ETH that are not his
  2. Attacker can prevent other user from using his PWays or getting back their ETHER (refund option)
  3. Attacker can take ownership of one of PWay smart contract
  4. Attacker can stop contract from normal operation, preventing people who wants to use PWay from doing so
  5. Any others that cause significant damage base on money/operational.

Major - 0.6 ETH all above with slight difference - is exists a workaround to a bug and do not require immediately code redeployment.

Low/Trivial Bugs 0 ETH bugs that do not cause lock or loss of funds will not be take into consideration.

For more details how this contracts are expected to work keep an eye on https://medium.com/pway analyze project's tests or visit https://pway.io and read also whitepaper

Bugs already found : @catageek - https://github.com/PwayGames/PWay.Contracts/pull/5 - 1 ETH @catageek https://github.com/PwayGames/PWay.Contracts/pull/11 - 0,7 ETH

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 5.0 ETH (1048.73 USD @ $209.75/ETH) attached to it as part of the PLAYWAY ESTONIA OÜ fund.

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 3 years ago. Please review their action plans below:

1) jakub-wojciechowski has started work.

It's my first time using gitcoin. I'll report issues one after another. 2) salmandabbakuti has started work.

primary goal is to check for the bugs in solidity programming language 3) catageek has started work.

I try to find the 'submit work' button but could not find it. Stopping to work and restarting is the last option I have

Learn more on the Gitcoin Issue Details page.

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 5.0 ETH (1070.5 USD @ $214.1/ETH) has been submitted by:

  1. @adamskrodzki
  2. @jakub-wojciechowski
  3. @catageek
  4. @catageek

@lukaszGrynasz please take a look at the submitted work:

PumpkingWok commented 5 years ago

Hi @adamskrodzki, I deleted my last comment because i was wrong about typo, sorry. For now i have only noticed that into DividendPayableToken.sol there is import 'openzeppelin-solidity/contracts/token/ERC20/MintableToken.sol';. This contract changed its name to ERC20Mintable.sol i think. It is not a real issue, just to let you know. Thanks.

lukaszGrynasz commented 5 years ago

Hi @adamskrodzki, I deleted my last comment because i was wrong about typo, sorry. For now i have only noticed that into DividendPayableToken.sol there is import 'openzeppelin-solidity/contracts/token/ERC20/MintableToken.sol';. This contract changed its name to ERC20Mintable.sol i think. It is not a real issue, just to let you know. Thanks.

Yes we use older version of openZeppelin our in SC's

jakub-wojciechowski commented 5 years ago

Anyone can start a new dividentPeriod as the function has a public access modifier:

function startNewDividendPeriod() public{

Moreover, it only checks if the period from the last invocation was no greater than 180 days, so it could be invoked immediately after claiming the funds:

require(getNow() - lastDividendTime < 180 days)

As a consequence the attacker will be able to clear the payments history:

clearDividends()

and request another dividend payment.

adamskrodzki commented 5 years ago

Anyone can start a new dividentPeriod as the function has a public access modifier:

function startNewDividendPeriod() public{

Moreover, it only checks if the period from the last invocation was no greater than 180 days, so it could be invoked immediately after claiming the funds:

require(getNow() - lastDividendTime < 180 days)

As a consequence the attacker will be able to clear the payments history:

clearDividends()

and request another dividend payment.

@jakub-wojciechowski

In order to get a bounty You should make PR with failing test

and submit work in gitcoin.co

startNewDividendPeriod is public by design (so anyone can start it, admin can not stop dividend from being paid)

require(getNow() - lastDividendTime < 180 days)

is a bug indeed should be require(getNow() - lastDividendTime > 90 days)

thats second from the list

web3.sha3("DividendPayableToken.sol line 63, should be getNow() - lastDividendTime>"+" "+" "+" "+"not '<', allows for more dividend than You are entitled to")

gives hash of

0xa3633a40fbdc1cddd982ed2483613e467229c549e477c20fe5961592ebe7dc0b

which is second from our list

0xa3633a40fbdc1cddd982ed2483613e467229c549e477c20fe5961592ebe7dc0b - 0,3 ETH Price

jakub-wojciechowski commented 5 years ago

Another one:

DIGITS_MULTIPLAYER = uint256(10)**(18-token.decimals());

is inconsistent with:

uint8 public decimals = 11;

I'm not sure which one is your true intention so please let me know, so I can classify the bug correctly.

jakub-wojciechowski commented 5 years ago

Next one:

function tranferTokens(address _beneficiary, uint256 _tokensToRelease, uint256 _weiAmount) public {

Is public so, anyone can call it passing any value for the _weiAmount and get the tokens. The access should be limited at least to the internal level.

adamskrodzki commented 5 years ago

Another one:

DIGITS_MULTIPLAYER = uint256(10)**(18-token.decimals());

is inconsistent with:

uint8 public decimals = 11;

I'm not sure which one is your true intention so please let me know, so I can classify the bug correctly.

What Do You mean by inconsistent?

DIGITS_MULTIPLAYER is value by which You need to multiply to compute PWAY from Ether (taking different number of decimals in ether and PWay)

since ether has 18 decimals and Pway has 11

You need to multiply by 10 000 000 to get price in same denomination

and value of uint256(10)**(18-token.decimals());` is 10 mln

If you still thinks it is a bug, proove by writing test, but I do not expect it to be here.

If you prove that it leads to unproportional payouts then that is 1 ETH

adamskrodzki commented 5 years ago

Next one:

function tranferTokens(address _beneficiary, uint256 _tokensToRelease, uint256 _weiAmount) public {

Is public so, anyone can call it passing any value for the _weiAmount and get the tokens. The access should be limited at least to the internal level.

web3.sha3("PwayKYCCrowdsale.sol line 120, tranferTokens should have private/internal instead of public")

hash is 0x9247b4c67bf0a167d79a1d94162b84fa90c90c8e8c3a26d4eff8fb38fb9f5a87

so You have found second hidden bug for 0.2 ETH

0.5 ETH total as for now.

Please write tests that exposes this bugs

lukaszGrynasz commented 5 years ago

Next one:

function tranferTokens(address _beneficiary, uint256 _tokensToRelease, uint256 _weiAmount) public {

Is public so, anyone can call it passing any value for the _weiAmount and get the tokens. The access should be limited at least to the internal level.

Congratulations ! At this speed , You will quickly find the third one :)

lukaszGrynasz commented 5 years ago

Next one:

function tranferTokens(address _beneficiary, uint256 _tokensToRelease, uint256 _weiAmount) public {

Is public so, anyone can call it passing any value for the _weiAmount and get the tokens. The access should be limited at least to the internal level.

@jakub-wojciechowski could You setup Your ETH address on Gitcoin ? We would like to send You a bounty.

For now in Your profile https://gitcoin.co/profile/jakub-wojciechowski

there is no address set

PumpkingWok commented 5 years ago

Hi @adamskrodzki , I would like to ask you a question. is there a specific reason why withdraw() is public in PwayGameStore.sol ? In this way everyone can call withdraw to transfer tokens to company and token address, right ? Thanks.

adamskrodzki commented 5 years ago

@PumpkingWok

Yes it is done to decentralize dividend so no one can stop from changing tokens collected in store to dividend. If it was onlyOwner, PWay would be able to stop this tokens from becoming dividend forever

now anyone who is willing pay for transaction can init the transfer

for the same reason startNewDividendPeriod() is public

and also finalize() in PwayAuthority.

Of course that do not mean there is no scenario which leads to an error there.

We did what we could to prevent it. But we are aware that introducing this level of decentralisation might leads to security breach.

This is what this bounty is for :)

lukaszGrynasz commented 5 years ago

Next one:

function tranferTokens(address _beneficiary, uint256 _tokensToRelease, uint256 _weiAmount) public {

Is public so, anyone can call it passing any value for the _weiAmount and get the tokens. The access should be limited at least to the internal level.

@jakub-wojciechowski 0xbc773ca86d9071e163168a8a5ad25e235907f9e7 is it Your ETH address, gitcoin have some issue on website , and we have to make sure we transfer found to right account ?

adamskrodzki commented 5 years ago

@catageek found an overflow bug in PWay.Company and wins 0.6 ETH (bug to be exploitet require access to one of 3 administrator identities)

gitcoinbot commented 5 years ago

⚡️ A tip worth 0.50000 ETH (109.72 USD @ $219.44/ETH) has been granted to @jakub-wojciechowski for this issue from @lukaszGrynasz. ⚡️

Nice work @jakub-wojciechowski! Your tip has automatically been deposited in the ETH address we have on file.

gitcoinbot commented 5 years ago

⚡️ A tip worth 1.00000 ETH (214.61 USD @ $214.61/ETH) has been granted to @catageek for this issue from @lukaszGrynasz. ⚡️

Nice work @catageek! Your tip has automatically been deposited in the ETH address we have on file.

adamskrodzki commented 5 years ago

Unfortunatelly there are some issues with @gitcoinbot and cooperative payment so we are giving payouts as tips

pi0neerpat commented 5 years ago

Which version of openZeppelin are you using. I have breaks in DividendPayableToken.sol import './openzeppelin-solidity/contracts/token/ERC20/MintableToken.sol'; I reverted to commit openZeppelin to commit 82b3ba before the MintableToken.sol was renamed to ERC20Mintable.sol, however this creates new issues. Just want to be sure I'm using the correct set of Zeppelin contracts.

lukaszGrynasz commented 5 years ago

Hi @blockchainbuddha we are using older version : "openzeppelin-solidity": "^1.12.0"

adamskrodzki commented 5 years ago

I've changed package.json

"openzeppelin-solidity": "^1.12.0"

to

"openzeppelin-solidity": "1.12.0" deleted openzeppelin-solidity folder from node_packages and performed npm install and that fixed the issue

try @blockchainbuddha

catageek commented 5 years ago

I made a PR but can't find the 'submit work' button anywhere.

lukaszGrynasz commented 5 years ago

@catageek can you provide any screenshot we forward it to gitcoin team

catageek commented 5 years ago

image

gitcoinbot commented 5 years ago

⚡️ A tip worth 0.70000 ETH (148.62 USD @ $212.31/ETH) has been granted to @catageek for this issue from @. ⚡️

Nice work @catageek! Your tip has automatically been deposited in the ETH address we have on file.

adamskrodzki commented 5 years ago

@blockchainbuddha do You work on this task?

pi0neerpat commented 5 years ago

@adamskrodzki Yes, I stopped after running into openzeppelin issue. Will continue working this week.

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This Bounty has been completed.

Additional Tips for this Bounty: