Smart Contract Audit

[thebkr7]

We are offering a bounty for finding bugs in this bounty smart contract. Please post your findings in this Github Issue along with you ETH address.

[nflaig]

I didn't find any particular bugs in the code of the smart contract but two issues that should be considered.

1. This is a minor issue but why does createBounty(uint256 _bountyId) take the _bountyId as input, shouldn't the ID of each bounty be managed internally by the contract and just incremented by 1 if a new bounty is created? Right now anyone can just create a bounty with any ID which still works but it seems kinda unnecessary and confusing.

2. Currently the creator/poster of a bounty can just use the rewardUser() function to send the reward to an address of himself. Therefore, the creator only has to pay the bountyFee even if someone submitted a valid solution. This kinda defeats the whole purpose of having the ether locked in the contract in the first place. A developer working on a bounty has to trust the creator to be a fair player and not just withdraw the locked ether after a solution is submitted. Edit: I just noticed that the bounty creator can do basically the same by using cancelBounty() which shouldn't even be possible after someone already submitted a solution in my opinion.

[catageek]

I found 4 vulnerabilities in your contract:

1. In rewardUsers(), someone can lock the rewards of any bounty if it throws on transfer() function (a contract with no default payable function for example). Since you use a loop to distribute rewards, anyone that throws on transfer() will lock the bounty reward. The address list will need to be manually cleaned to remove the faulty address. You should use a pull pattern where users will need to fetch the reward themselves.
2. Once you will have fixed this, an interger overflow in rewardUsers(), line 152, will be exploitable. It allow anyone who has created a bounty to empty the contract account or transfer all its token. If someone calls rewardUsers() with a specially crafted _rewards array, currentRewards may overflow and may be nullified. The check of line 155 will pass and arbitrary amounts of rewards may be transferred. Check that the addition line 152 does not overflow
3. The owner can steal all the ethers of the contract because there is a possible integer overflow line 203. If the owner of the contract sets minBounty to 2**32-bountyFee, the check line 203 will overflow and allows the creation of a bounty with msg.value = 0. The computation of bounty.bounty will underflow, like bounty.remainingbounty who will be a very large value. A call to rewardUser() with an arbitrary _reward value will pass because the check against bounty.remainingbounty will be ineffective. _reward can be set to the value of the balance of the contract to empty it.
4. The same vulnerability is present line 226 with the same effects on tokens.

My ETH address is 0x00f9474f1f603de1c3ba6fbe525de1ffb389125b

[thebkr7]

@catageek @nflaig You've both been rewarded. Sorry for the delay.