moonshotcollective / pay.party

pay-party.vercel.app
MIT License
9 stars 18 forks source link

Some potential security concerns for the smart contracts #44

Closed asgeir-s closed 2 years ago

asgeir-s commented 3 years ago

To my understanding, any account can withdraw any ether held by the contract. Via calling the Diplomat.payElection with:

electionId = anyNumber, 
_adrs = [attackeAddress], 
_shares = [numberOfEthInTheContract], 
_token= 0x0000000000000000000000000000000000000000.

To my understanding, this is because in Distributor._sharePayedETH there is no check for msg.value. I think that there needs to be some check that the sum of shares is smaller or equal to the msg.value.

By introducing, the fix the possibility of depositing ETH into the contract seems redundant. I would therefore remove the functions for depositing ETH: Diplomat.deposit, Diplomat.receive, and Diplomat.fallback.

There is also a lot of unused code and perhaps unnecessary complexity in the Distributor contract. I would perhaps also refactor the contract to not depend on the generic Distributor._distribute function in favor of less generic and less complex code.

hmrtn commented 2 years ago

Contract has been reworked. Always open to more review.