PoolTogether v2.0: Audit Disclosures
Last week OpenZeppelin Security completed a comprehensive audit of our smart contracts. Their analysis found no serious issues, but did yield a number of areas that PoolTogether can improve.
We’ve highlighted our centralization in the past, but we’d like to do so again to maintain full transparency with the community.
The three major areas for improvement are contract upgradeability, the privileged administrator role, and our use of ERC777.
Contract Upgradeability
Contract upgradeability means that whoever controls the contract can replace it with any code of their choosing. We designed the protocol this way so that we can fix bugs and add features, but ultimately our intention is to be able to step away from a Pool and have it run autonomously.
The “upgrade controller” of our contract is a 2-of-N Gnosis Multisig whose members are the founding PoolTogether team. In order to upgrade the contract, two members of the team must approve the upgrade in a multisig ceremony. This mitigates the risk of one team member being compromised; an attacker would need to compromise at least two keys in order to take control of the contract. We believe this is sufficient security for the time being, but have plans for additional measures as we grow.
Privileged Administrator Role
When we first designed PoolTogether, we wanted the user experience to be as simple and seamless as possible. We opted to have an “admin” that would take care of the day-to-day running of the Pool. Each of the PoolTogether co-founders are admins of the Pool. Our privileged actions include prize control and parameter control.
Prize Control
The admins are the only accounts that can open the next draw or award the draw prize. When a draw is opened, the admin commits the hash of a salted secret. When the admin rewards the draw prize, they reveal the secret and the salt. The hash of the secret is used as the entropy to randomly select the winner. This is the most centralized aspect of the system: the admin is the only one contributing to the random number. This is something we’re looking to address soon, especially as we begin to open up the protocol and allow others to create their own Pools.
Parameter Control
The Pool has the ability to extract a fraction of the winnings as a “fee”. The fee has two parameters: the fraction percent and the beneficiary address. The fraction and beneficiary are set for each draw when it is opened. If they are changed, the changes will apply to the next draw; this helps mitigate last-minute changes to prevent admins from diverting funds.
Originally, 10% of the interest earned on each pool was redirected back to the pool contract. In December this was adjusted to 0% and will remain at 0%.
ERC777
PoolTogether is now tokenized, and we decided to implement the ERC777 standard. We opted for this standard because it is backwards compatible with the common ERC20 standard, has very useful features, and a solid OpenZeppelin implementation exists.
Receive Hooks
The ERC777 spec has been dismissed by some in the community as insecure due to its most powerful feature: receive hooks. Receive hooks allow a token recipient to trigger a smart contract when they receive tokens. It’s a wonderful tool that has allowed us to easily extend PoolTogether with cool new features. However, the biggest problem is that it opens the contract up to reentrancy issues. Our code is protected using reentrancy guards, but other contracts that integrate with our token will also need to be cognizant of reentrancy exploits.
Deviation from Spec
Another issue with our implementation of ERC777 is that we deviate from the spec. The spec requires that we emit “Minted” events when new tokens are minted. However, we mint tokens en-masse when a draw is opened and all of the players’ deposits are converted to tickets. It wasn’t feasible for us to emit an event for each player, so we emit a single Minted event for the Pool itself.
Allowance Double Spend Attack
Our final change to our implementation of ERC777 was the addition of the functions `increaseAllowance` and `decreaseAllowance`. These functions are superior to the standard ERC20 `approve` function in that they aren’t subject to double spend attacks. More details can be found in the audit.
Summary
We’re very excited by the interest and growth that PoolTogether has seen over the last year. We take our responsibility of the protocol very seriously, so addressing any security concerns is a very high priority for us. We have increased the size of our bug bounties so if you spot anything let us know!
In 2020 we will be looking to further decentralize the protocol; not just for security but to increase its utility to the ecosystem.
https://www.pooltogether.com/
Twitter: @PoolTogether_
Github: pooltogether
Discord: https://discord.gg/hxPhPDW
